add: ruff linter

[virajkanwade]

poetry run ruff check --fix appmap

https://docs.astral.sh/ruff/

[apotterri]

We're already using pylint for linting: https://github.com/getappmap/appmap-python/blob/master/tox.ini#L24 . I'm not opposed to switching to ruff, but if we do, we should remove pylint. This should probably also include a config file for ruff that more or less matches the pylint config.

[apotterri]

We're using semantic-release for appmap-python, so the subject lines for commits need to follow this format: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#-commit-message-format .

(Our own CONTRIBUTING.md really needs an update that mentions this....)

[apotterri]

Thanks very much for this.

[apotterri]

This import needs to be retained, so it should be marked with # noqa: F401 rather than deleted.

[virajkanwade]

Fixed

[apotterri]

This should be in the dev dependencies (below).

[virajkanwade]

We're using semantic-release for appmap-python, so the subject lines for commits need to follow this format: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#-commit-message-format .

(Our own CONTRIBUTING.md really needs an update that mentions this....)

Updated the commit message. Hopefully chose the correct one.

[virajkanwade]

We're already using pylint for linting: https://github.com/getappmap/appmap-python/blob/master/tox.ini#L24 . I'm not opposed to switching to ruff, but if we do, we should remove pylint. This should probably also include a config file for ruff that more or less matches the pylint config.

Most of projects are now moving from pylint to ruff.
https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-pylint

Also, the fastapi unused wsgimiddleware import, ruff highlighted it, but I didn't notice it as such in the pylint output.

This was a suggestion for replacing pylint with ruff and not both together.

I am not aware of other places where you might have to replace pylint in the dev environments/ CI etc.

[apotterri]

We're already using pylint for linting: https://github.com/getappmap/appmap-python/blob/master/tox.ini#L24 . I'm not opposed to switching to ruff, but if we do, we should remove pylint. This should probably also include a config file for ruff that more or less matches the pylint config.

Most of projects are now moving from pylint to ruff. https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-pylint
Yup, seems like a good idea to me.

Also, the fastapi unused wsgimiddleware import, ruff highlighted it, but I didn't notice it as such in the pylint output.

This was a suggestion for replacing pylint with ruff and not both together.
Understood. I wasn't suggesting this either.

I am not aware of other places where you might have to replace pylint in the dev environments/ CI etc.
The main thing we're using pylint to try to keep the code from getting worse: https://github.com/getappmap/appmap-python/blob/master/tox.ini#L24 .

We need to retain this functionality if we switch to ruff, so this PR would need to include changes to do that, as well.

[virajkanwade]

@apotterri Just to confirm, this is executed when running tox? Also, what is the expected outcome here? Tests should fail if there is any pylint error? As pylint-exit only returns 1 on Fatal message issued or Usage error

[apotterri]

Yes, it's run when the tox environment is py310-web: https://app.travis-ci.com/github/getappmap/appmap-python/jobs/619305807#L579 .

pylint computes a score for the code, and has a setting that controls the minimum score: https://github.com/getappmap/appmap-python/blob/master/pylintrc#L3 . If the code that's getting built has a score that's below that threshold, pylint-exit returns non-zero.

[virajkanwade]

Yes, it's run when the tox environment is py310-web: https://app.travis-ci.com/github/getappmap/appmap-python/jobs/619305807#L579 .

pylint computes a score for the code, and has a setting that controls the minimum score: https://github.com/getappmap/appmap-python/blob/master/pylintrc#L3 . If the code that's getting built has a score that's below that threshold, pylint-exit returns non-zero.

Doesn't seem to be working that way.

# Specify a score threshold under which the program will exit with error.
fail-under=9.83

------------------------------------------------------------------
Your code has been rated at 9.80/10 (previous run: 9.80/10, +0.00)

The following messages were raised:

  - error message issued
  - warning message issued
  - refactor message issued
  - convention message issued

No fatal messages detected.  Exiting gracefully...

[apotterri]

That would be bug, then. It's intended to work that way, and I thought that was the description from pylint-exit.

[apotterri]

That would be bug, then. It's intended to work that way, and I thought that was the description from pylint-exit.

Looking again at the doc for pylint-exit, I see that I misread it. It doesn't clean to exit non-zero if the fail_under check fails. So, there's definitely a bug in the build that needs to be fixed.

[apotterri]

As far as replacing pylint with ruff....

I'm concerned about the statement here that ruff isn't a drop-in replacement for pylint: https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-pylint , and especially that it does less type checking.

Also astral-sh/ruff#970 .

[virajkanwade]

Yes, ruff is used along with mypy. But at this time, adding type annotation for every variable might be a big task. We can leave adding ruff in backlog as a future task, but address the highlighted issues in a separate PR.