Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cEP 23: Breaking down coala #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 277 additions & 0 deletions cEP-0023.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
# Breaking down coala

| Metadata | |
|----------|-----------------------------------------------|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -1,7 +1,7 @@
 # Breaking down coala
 
 | Metadata |                                               |
-|----------|-----------------------------------------------|
+| -------- | --------------------------------------------- |
 | cEP      | 0023                                          |
 | Version  | 1.0                                           |
 | Title    | Breaking down the coala                       |

| cEP | 0023 |
| Version | 1.0 |
| Title | Breaking down the coala |
| Authors | Muhammad Kaisar Arkhan <mailto:[email protected]> |
| Status | Proposed |
| Type | Feature |

## Abstract

This cEP proposes a new architecture evolution for coala.

## How coala works

coala is a Python program that also provides API for bears and interacting with
them.

Bears are Python classes filled with metadata needed to execute linter programs
and may contain helper code to generate configuration files. Since bears are
Python code, some bears have the actual linter inside of them and some just
execute the Python API of a linter.

Bears are written with the API provided by the coala core and they're running
under the same process as coala.

On startup, coala reads and intepret the configuration file. Configuration files
are separated by sections which contains what bear is required and the
configuration for the bear.

After it's intepreted, it is passed over to an executor which will executes it
per section. The executor will find and import the bear class which usually
resides in the Python packages path. It then pass the Section data to the bear
and set it's require configuration. The bear first prepare the whole command
along with it's parameters or generate a configuration file. After that's done,
the linter program will be executed by the bear as a separate process and the
bear will intepret it's standard output by matching a regex. The bear will yield
the output of the program to the executor which will end up to the generator
where the user see the warning/error and determine an action.

## The Architectural Issue

There is no clear distinction between the coala core which job is to intepret
and run bears and the one that shows the command line interface to the user. In
addition, bears in essence can do anything it wants to since it is just a Python
class that is loaded into the coala process.

This caused numerous unwanted regressions between bears and coala and costed a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an example here? What regression/problem did the current architecture bring about?

lot in maintenance. This is also the reason why bears are not released
separately as a "collection" rather it is shipped with coala core along with
every linter in the collection.

Instead of leaving the project at this state, we should define borders and setup
a more proper architecture and standards.

## The new architecture

yukiisbored marked this conversation as resolved.
Show resolved Hide resolved
Bears are now just simple formatted plain-text file containing the metadata
needed. It can also contain a simple program as helpers or sole linters.

coala will separated into two parts: The coala Engine and The coala CLI.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be separated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of one coala process, there will be two separate processes in the new architecture, is my understanding correct? Maybe this paragraph can be elaborated a bit more :)


The coala CLI's sole job is to present an interactive command line interface for
the coala Engine. The CLI will present the output of the Engine and present them
with possible actions that the user can do.

Meanwhile, the coala Engine's sole job is to intepret the bear files and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a typo: intepret

execute/supervise the linter/helper programs along with committing the action
that is requsted by the user.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a typo here :P


If a helper program is needed, They'll act as the linter program and do stuff
before the program (e.g generating the configuration file) and do stuff
after it (e.g cleaning).

The frontend and backend programs will communicate to each other via json-rpc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not very clear which programs frontend and backend are referring to. I assume frontend = coala CLI and backend = coala Engine, but it would be better to be more precise.

json-rpc is chosen for it's simplicity.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One advantage of using RPC I can think of is: in the future the coala Engine might be able to run on the cloud and users only need a coala CLI running locally.

But do we need an RPC framework if we only use it for local interprocess communication?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use JSON-RPC because it is easier than other IPC/RPC implementations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we want it to be easily used by other programs such as plugins for a text editor or maybe a GUI version of the frontend.


## The new bear file

Bears will be ini files which is already a part of the Python standard library.
Ini files are readable and simple to write. Making it a lot easier for projects
making their own internal bears.

### GoVet.bear

```ini
[bear]
name = GoVetBear
description = Analyze Go code and raise suspicious constructs, such as printf
calls whose arguments do not correctly match the format string,
useless assignments, common mistakes about boolean operations, unreachable code,
etc.
languages = Go
authors = The coala developers
authors_email = [email protected]
license = AGPL-3.0
can_detect = Unused code, Smell, Unreachable Code

[requirement]
type = go
package = golang.org/cmd/vet

[run]
executable = go
arguments = vet
use_stdout = false
use_stderr = true
output_regex = .+:(?P<line>\d+): (?P<message>.*)
```

The coala Engine will intepret the following file, check the requirement, and
execute the linter program while intepreting the output with the regex.

### SpaceConsistency.bear

```ini
[bear]
name = SpaceConsistencyBear
description = Check and correct spacing for all textual data. This includes usage of
tabs vs. spaces, trailing whitespace and (missing) newlines before
the end of the file.
languages = All
authors = The coala developers
authors_email = [email protected]
license = AGPL-3.0
can_detect = Formatting

[param.use_spaces]
description = True if spaces are to be used instead of tabs
type = boolean

[param.allow_trailing_whitespace]
description = Whether to allow trailing whitespace or not.
type = boolean
default = false

[params.indent_size]
description = Number of spaces per indentation level
type = int
default = 8

[params.enforce_newline_at_EOF]
description = Whether to enforce a newline at the end of file
type = boolean
default = true
config_key = enforce_newline

[run]
local = true
executable = space_consistency_bear.py
output_regex = .+:(?P<line>\d+): (?P<message>.*)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -151,7 +151,6 @@
 local = true
 executable = space_consistency_bear.py
 output_regex = .+:(?P<line>\d+): (?P<message>.*)
-

The coala Engine will intepret the following file, setup the command arguments,

```

The coala Engine will intepret the following file, setup the command arguments,
and run the Python file. Note that it does not load it as a library rather
running it as a program.

This also means it is possible to run it without the engine.

```
/usr/local/share/coala/bears/SpaceConsistency/space_consistency_bear.py \
--use_spaces=false \
--allow_trailing_whitespace=false \
--indent_size=8 \
--enforce_newline_at_EOF=8 \
```

## The RPC

The RPC will follow the JSON-RPC specification
(https://www.jsonrpc.org/specification).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -171,7 +171,7 @@
 ## The RPC
 
 The RPC will follow the JSON-RPC specification
-(https://www.jsonrpc.org/specification).
+(<https://www.jsonrpc.org/specification>).
 
 The methods will be the following:
   * `start`


The methods will be the following:
* `start`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -174,10 +174,12 @@
 (https://www.jsonrpc.org/specification).
 
 The methods will be the following:
-  * `start`
-  * `commit`
+
+- `start`
+- `commit`
 
 The following examples will use the syntax below:
+

--> data sent to the Engine
<-- data sent to the Frontend

* `commit`

The following examples will use the syntax below:
```
--> data sent to the Engine
<-- data sent to the Frontend
```

### `start_session`

`start_session` will start a new coala session. It will pass the whole
configuration.

```json
--> {
"jsonrpc": "2.0",
"method": "start_session",
"params": {
"sections": [
"all": {
"bears": ["SpaceConsistencyBear"],
"use_spaces": true,
"files": "**/**.py"
}
]
}
}
<-- {
"jsonrpc": "2.0",
"result": {
"file": "/home/asuka/src/myproject/main.py",
"bear": "SpaceConsistencyBear",
"message": "Tabs used instead of spaces.",
"line": 47,
"patch": "<patch goes here>",
"has_next": true,
"actions": [
{
"action": "patch",
"name": "Apply Patch",
"description": "Applies the proposed patch to the file"
},
{
"action": "ignore",
"name": "Ignore",
"description": "Ignore the warning"
}
]
}
}
```

### `commit`

`commit` will commit an action proposed by the Engine and pass onto the next warning.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line must be at most 80 characters maximum-line-length remark-lint

Origin: MarkdownBear, Section: markdown.


```json
--> {
"jsonrpc": "2.0",
"method": "commit",
"params": {
"action": "ignore"
}
}
<-- {
"jsonrpc": "2.0",
"result": {
"file": "/home/asuka/src/myproject/handler.py",
"bear": "SpaceConsistencyBear",
"message": "Tabs used instead of spaces.",
"line": 20,
"patch": "<patch goes here>",
"section": "all",
"has_next": false,
"actions": [
{
"action": "patch",
"name": "Apply Patch",
"description": "Applies the proposed patch to the file"
},
{
"action": "ignore",
"name": "Ignore",
"description": "Ignore the warning"
}
]
}
}
--> {
"jsonrpc": "2.0",
"method": "commit",
"params": {
"action": "ignore"
}
}
<-- {
"jsonrpc": "2.0",
"result": {}
}
```