This has a ton of exclude rules, for two reasons:
(1) We haven't been particularly systematic about avoiding unnecessary
inline style in the past, so there's a lot of code we need to fix.
(2) There are cases where one wants to dynamically compute style
rules. For the latter category, ideally we'd figure out a way to
exclude these automatically (e.g. checking for mustache tags in the
style tag).
These are long enough to still be self-explanatory (the only one I'm
at all in doubt about there is DEBG; I avoided "DBUG" because it reads
"BUG" which suggests a high-priority message, and those are the
opposite of that), while saving a good bit of horizontal space
vs. padding everything to the 8 characters of "CRITICAL".
Also add a linter exception to allow easy-to-read alignment here,
similar to several existing exceptions for other alignment cases.
This doesn't yet do much, but it gives us a suitable place to
add code to customize how log messages are displayed, beyond what
a format string passed to the default formatter can do.
The pattern test method `test_rule_patterns` tests each rule by
fetching two strings from it: `test_good` and `test_bad`. Each
string is then presented as an input file to `custom_check_file`,
which should return True or False.
All lines in a string need to end with `\n`. Since the linter
expects an additional newline at the end of a file, the test case
adds `\n` to each string on top of that.
Fixes#6320.
This enforces our use of a consistent style in how we access Python
modules; "from os.path import dirname" is a particularly popular
abbreviation inconsistent with our style, and so it deserves a lint
rule.
Commit message and error text tweaked by tabbott.
Fixes#6543.
We want to convert stream names to stream ids as close
to the "edges" of our system as possible, so we let our
caller do the work of finding the stream id for a stream
narrow.
There are four regexes which try to ensure that the i18n strings are
properly captured.
1) The one which disallows multiline strings.
```
i18n\.t\([^)]+[^,\{\)]$
// Disallows:
i18n.t('some '
+ 'text');
```
2) The one which disallows concatenation within argument to i18n.t():
```
i18n\.t\([\'\"].+?[\'\"]\s*\+
// Disallows:
i18n.t("some " + "text");
```
3) There are two which disallow concatenation with i18n.t():
```
i18n\.t\(.+\).*\+
// Disallows:
i18n.t('some text') +
\+.*i18n\.t\(.+\)
// Disallows:
+ i18n.t('some text')
```
The ideal case is that you try to bring the string argument to the
i18n.t() on one line. In case this is not possible, you can do the
following:
```
var1 = i18n.t("Some text to be translated");
var2 = i18n.t("Some more text to be translated");
complete = var1 + var2;
This is needed in order to mock the method when testing
`custom_check.py`. The diff for this commit is a bit broken;
all it really does is moving the method out of `build_custom_checkers`.
We were getting pyflakes lint error output without line numbers like
this:
pyflakes | if user_profile.is_realm_admin and
pyflakes | ^
pyflakes |
Apparently the cause was that stdout and stderr was getting mixed
badly, creating "unused import"s lines that had the first of that
error (containing the line number) just above.
As a result, printing out the lines of output from pyflakes' merged
stdout/stderr feed looked like this:
b"zproject/settings.py:95: 'from .prod_settings import *' used; unable to detect undefined nameszerver/views/users.py:49:39: invalid syntax\n"
Note the lack of newline in between the end of the first error at
"names" and the start of the second at "zerver".
This appears to be a change in Pyflakes behavior when we switched to
Python 3; probably they're missing a flush() somewhere.
We're about to do this (a) in a number of places mentioning
system packages like `python3-dev` to install, and (b) in the
shebangs of every script in the tree.
In preparation for the upcoming linter output update, each custom
linter now has a identifier argument. This would be used for separating
each line of lints, eg:
py | Missing space around "%" at analytics/views.py line 485:
py | output += '<hr>%s\n'%(string_id,)
css | 64c64,65
css | < opacity: 1; }
css | ---
css | > opacity: 1;
css | > }
css | static/styles/lightbox.css seems to be broken:
swagger | In static/swagger/zulip.yaml:
swagger | Duplicate operationId: registerQueue
For performance reasons, we spawn each linter in a separate OS thread.
The downside of this is that all lints would end up in stdout without
much visual separation, resulting in confusing error log. This commit
introduce the `print_err` function, which shows which linter each line
of lint is from.
In order for the `include_only` linter rule to not have
any side effects, we need to explicitly add a trailing '/'
after every directory we want to include.
The main thing here is to make looping over lines be the inner
loop, instead of looping over rules. This keeps regexes in
cache, and it also avoids some O(N) checks.
This is a significant speedup for me, reducing time from 16s
to 11s.
As part of extracting this, we exempt the library from all custom
checks on itself. This is expedient, since a lot of our
custom checks are naive about whether things are in strings, and
it is also a pain to configure individual rules.
derAnfaenger
Greg Price
Juliana Bacelar
julianasobreira
Steve Howell
Supermanu
Umair Khan
Tim Abbott
Yago González
Rishi Gupta
Greg Price
Jack Zhang
neiljp (Neil Pilgrim)
rht
Aditya Bansal
Tommy Ip
Eeshan Garg
Steve Howell
vaibhav