Skip to content

Commit

Permalink
Guide: address common lint errors (#10050)
Browse files Browse the repository at this point in the history
Developers are having trouble to know how to resolve certain lint errors.

Specifically misleading is ET128 (flake8-tabs) unexpected number of tabs and spaces at start of expression/definition line. It is likely that flake8 is expecting a code alignment style, rather than hanging indent style in this case. To make it expect hanging indent, first ensure that there is a newline after the opening paren/bracket/brace.
  • Loading branch information
feerrenrut committed Aug 12, 2019
1 parent b4db7dc commit 7085172
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 0 deletions.
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -256,6 +256,7 @@ scons checkPot

### Linting your changes
In order to ensure your changes comply with NVDA's coding style you can run the Flake8 linter locally.
Some developers have found certain linting error messages misleading, these are clarified in `tests/lint/readme.md`.
Running via SCons will use Flake8 to inspect only the differences between your working directory and the specified `base` branch.
If you create a Pull Request, the `base` branch you use here should be the same as the target you would use for a Pull Request. In most cases it will be `origin/master`.
```
Expand Down
111 changes: 111 additions & 0 deletions tests/lint/readme.md
Expand Up @@ -4,6 +4,117 @@ Our linting process with Flake8 consists of two main steps:
- Generating a diff
- Running Flake8 on the diff

## Common problems
There are several common situations for which the Flake8 errors don't clearly indicate an error free solution.

### Continuation lines

According to the
[Pep8 indentation guide](https://www.python.org/dev/peps/pep-0008/#indentation),
continuation lines (Python's implicit line joining inside parentheses) are expected
to follow one of two main styles:

- Vertical alignment
- The first parameter is on the same line as the opening parenthesis
- For each subsequent line, the first character should be aligned with the
first character of the first parameter.
- Hanging indent
- There is no parameter on the same line as the opening parenthesis.
- The first parameter is indented by a standard amount on the following line.
- All subsequent parameters have the same indent.

What this means for us:
- Vertical alignment
- Requires spaces to meet the length of arbitrary function/variable names
- Requires counting characters to determine the number of spaces before arguments
- Alignment must be changed if the function/variable name changes.
- Hanging indent
- Fixed indentation
- Takes up more vertical space

The Flake8 checkers don't know which style we intend, they allow for both. It guesses
which one you want based on the location of the first token after the opening parenthesis.
The expected indentation it suggests can cause confusion, especially if the indentation is
as expected, but the first param/condition/item is on the same line as the opening parenthesis.

#### Preferred formatting for continuation lines

- Use hanging indent style.
- Line break after the opening parenthesis, putting the first parameter on a new line.
- For function definitions, double indent the params to avoid ET121

```python
# method with many parameters
# use "hanging indent style" - start params on new line to avoid ET128
def foo(
arg1, # double indent to avoid ET121
arg2
): # put the closing paren on a new line, reduce the diff when changing parameters.
# long expression
# use "hanging indent style" - start params on new line to avoid ET128
if(
arg1 is not None # not a function definition, no double indent required
and arg2 is None
): # put the closing paren on a new line, reduce the diff when changing conditions
return None

# use "hanging indent style" - start params on new line to avoid ET128
values = [
"value1", # not a function definition, no double indent required
"value2",
] # put the closing bracket on a new line, reduce the diff when adding items.
return values

```

Note: An inline comment on an opening parenthesis/bracket/brace causes an
erroneous message from flake8-tabs. See https://gitlab.com/ntninja/flake8-tabs/issues/1
EG:
```python
def foo( # a comment here causes error ET128
arg1
):
items = [ # a comment here causes error ET128
"item1",
"item2",
]
```

#### ET128 (flake8-tabs)

Error messages:
- *unexpected number of tabs at start of definition line*
- *unexpected number of tabs and spaces at start of expression line*s

Its likely that this is triggered because the linter is expecting "vertical alignment"
style for the set of continuation lines, rather than "hanging indent" style. To change this,
ensure that there is a newline after the opening paren/bracket/brace.

An example cause:
```python
def foo(arg1,
arg2, # arg2 not vertically aligned with start of first parameter.
):
return None
```

#### ET121 (flake8-tabs)

Error messages:
- *unexpected number of tabs at start of definition line (expected 2, got 1)*

Example cause:
```python
def foo(
arg1, # one level of indentation, matches the function body
arg2,
):
return None
```

In function definitions we require double indentation of parameters to differentiate from
the body of the function.

## Scons lint
Executed with SCons.
```
Expand Down

0 comments on commit 7085172

Please sign in to comment.