Our use case is that we are running many commands through linty fresh - for our iOS repo, we have xcodebuild, swiftlint, and stringslint errors.

When one of these fails, people have ended up being confused about the difference between a "lint" step - "oh, stupid lint" ... except that the code is not compiling ("xcodebuild").

I've updated the comments to be more specific. For example rather than saying "Linty Fresh Says" now we'll say "xcodebuild says".

By default the name of the --linter will be used. I also added a --linter_name argument which can be used to override this in cases where it might not make sense (e.g. passthrough)

Hoping this fixes this error:

Traceback (most recent call last):
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/lyftios/tools/lintyfresh/linty_fresh_runner.py", line 9, in <module>
    linty_fresh.main.main()
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/main.py", line 95, in main
    loop.run_until_complete(run_loop(args))
  File "/Applications/Xcode-12.2.0.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/main.py", line 87, in run_loop
    await asyncio.gather(*awaitable_array)
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/reporters/github_reporter.py", line 82, in report
    with aiohttp.ClientSession(headers=headers) as client_session:
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__aiohttp/aiohttp/client.py", line 1070, in __enter__
    raise TypeError("Use async with instead")
TypeError: Use async with instead

This reverts commit 9aa2b7621872dcadc80538d56a3a6cef2abdbb55.

When running after this change we see:

Traceback (most recent call last):
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/lyftios/tools/lintyfresh/linty_fresh_runner.py", line 9, in <module>
    linty_fresh.main.main()
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/main.py", line 95, in main
    loop.run_until_complete(run_loop(args))
  File "/Applications/Xcode-12.2.0.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/main.py", line 87, in run_loop
    await asyncio.gather(*awaitable_array)
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__linty_fresh/linty_fresh/reporters/github_reporter.py", line 82, in report
    with aiohttp.ClientSession(headers=headers) as client_session:
  File "/private/var/tmp/b/3263360e4c13ed7221fd5574dfffe441/execroot/lyftios/bazel-out/darwin-opt/bin/tools/lintyfresh/lintyfresh.runfiles/pip_deps/pypi__aiohttp/aiohttp/client.py", line 1070, in __enter__
    raise TypeError("Use async with instead")
TypeError: Use async with instead

Would it make sense to delete all existing linty_fresh comments when making new ones? Sometimes these comments are folded, depending on the linter, but they're also sometimes not, or at the top level which makes it hard to tell if the comment has been addressed.

I don't feel like these automated lint comments add a ton of historical value so if re-running the linter has the capability of making the same comments, if the issues haven't been addressed, it seems like we would be safe first clearing the existing ones.

In SwiftLint there are rules that can be enforced like the length of a file. When these hit, the error message will be on the last line even though the commit caused the failure.

This will still comment if it can on the top of the file with any failing messages.

Update:

I'm sorry I wanted to do it on my master branch :(

(Forgot a minute that github automatically use the fork source as the source of the PR :( )

We need the path and the line to create the message, and the regex for the pylint linter does not ensure it.

In a pylint report, it is possible to find a line containing only :: so the script crashes.

This commit fixes the problem by updating the PYLINT_LINE_REGEX to force the presence of a path and a line.

This removes our usage of setup_requires for development only dependencies, and manages linters in a way that I think is nicer in general. This fixes an issue we hit where setup_requires, even though it's not consumed upstream, broke upstream dependees because of invalid version locking.

This is the newer / nicer way to configure this without arbitrary code execution. I was hoping this would help with our install issues, but it doesn't look like it will. Either way I think it's the right thing to do

flake8-debugger shipped a new version that's incompatible with the other constraints. In the future we probably want to change how this works but for now this fixes that issue.

Before this commit, nothing was printed on the output to know if something happened or not, and there was no difference in the exit code if lint problems were found or not.

The exit code statis is important when integrating this tool in a CI, to mark the PR as failed.

This commit resolves this by:

• adding a return value to the report method, a tuple with:
  • Reporter name
  • Number of line with problem(s)
  • Number not already present on github
  • Raised overflow, or None
  • Number that cannot be pushed on github
• passing this result from the main function to a new manage_results function that will:
  • display a message depending of the result for each reporter
  • return 1 if at least 1 problem was found in the PR (even if already pushed on Github on a previous call), and 0 if not

To know if a message should be added to Github we check if it was already added. Or if we have more than one messages for a line, they are not guaranteed to be in the same order, so the check fail.

This commit simply sort the lines before adding them to the final message to check.

(It also simplifies this part of the code using the fact that set.add will not add a new occurence of the value if already present, so the extra set and the if..in are not needed)

FindBugs is a popular Java static analyzer. http://findbugs.sourceforge.net/

See our guidelines for contributing for more info on how to add a linter https://github.com/lyft/linty_fresh/blob/master/CONTRIBUTING.md

http://fbinfer.com/ is a great static analyzer for finding bugs. We should support parsing it's output.

For example:

./Root/Hello.java:27: error: NULL_DEREFERENCE
  object a last assigned on line 25 could be null and is dereferenced at line 27

./Root/Hello.java:37: error: RESOURCE_LEAK
   resource acquired to stream by call to allocateResource() at line 31 is not released after line 37

See https://github.com/lyft/linty_fresh/blob/master/CONTRIBUTING.md for more information on how to get started.