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

[CHIA-1553] Replace pylint with ruff #18759

Merged
merged 7 commits into from
Oct 25, 2024
Merged

Conversation

Quexington
Copy link
Contributor

@Quexington Quexington commented Oct 23, 2024

Blatant plagiarism of #18649 with less emojis and politics :)

Ruff is a python linter and formatter written entirely in Rust that aims to reimplement many of the most popular tools in a way where they can run hundreds of times faster. It's used to lint the source of many major projects like Mypy, Poetry, PyInstaller, pytest, and even Pylint, which this PR hopes to replace.

The introduced scope of ruff here is very limited compared to what it can do. We're only replacing pylint in this PR even though this tool could in theory absorb 4 or 5 more of our pre-commit jobs (and run them all in a fraction of the time). The reason for targeting pylint is two-fold: 1) pylint cannot run in pre-commit because it's so slow. This often results in unexpected CI failures that devs would have much rather caught at commit time. 2) pylint at once wants to check type related information and at the same time is pretty bad at it. This is a known issue in pylint that results in many false positives and while they're working to improve it, they are slow to respond to false positive issues and have seeming architectural issues that prevent them from addressing some of the more annoying failures like with respect to wrappers and dataclasses.

Ruff does not have exact 1:1 parity with pylint, however, they are working in that direction: astral-sh/ruff#970. They also don't attempt type inference that they're bad at, which means we're trading some linting coverage for less false positives and pylint: disable= statements in our codebase.

In terms of Ruff ergonomics, many errors can be fixed automatically (and are by pre-commit) and errors that cannot be fixed give much more descriptive formatting to help developers solve them. Ignoring ruff errors works like flake8 (because it can replace it) by saying # noqa: <error code>.

Copy link

socket-security bot commented Oct 23, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1, pypi/ruff@0.7.1

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Oct 23, 2024
@Quexington Quexington force-pushed the quex.replace_pylint_with_ruff branch from e8ca12b to fcd342e Compare October 24, 2024 16:45
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Oct 24, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 24, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 24, 2024
@Quexington Quexington force-pushed the quex.replace_pylint_with_ruff branch from fcd342e to 4829ecb Compare October 24, 2024 17:18
Copy link

socket-security bot commented Oct 24, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
pypi/ruff@0.7.1 environment, eval, filesystem, network, shell, unsafe 0 36.2 MB astral-bot, crmarsh, zanie

🚮 Removed packages: pypi/astroid@3.2.4, pypi/dill@0.3.7, pypi/pylint@3.2.6, pypi/pylint@3.3.1, pypi/tomlkit@0.12.1

View full report↗︎

@Quexington Quexington added the Added Required label for PR that categorizes merge commit message as "Added" for changelog label Oct 24, 2024
@Quexington Quexington marked this pull request as ready for review October 24, 2024 17:37
@Quexington Quexington requested review from altendky and a team as code owners October 24, 2024 17:37
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Oct 24, 2024
@Quexington Quexington force-pushed the quex.replace_pylint_with_ruff branch from 4829ecb to 6b4e8eb Compare October 24, 2024 19:14
@Quexington Quexington force-pushed the quex.replace_pylint_with_ruff branch from 6b4e8eb to ac00bdb Compare October 24, 2024 19:20
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 24, 2024
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Oct 24, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 24, 2024
@emlowe
Copy link
Contributor

emlowe commented Oct 24, 2024

@SocketSecurity ignore pypi/ruff@0.7.1

@wallentx
Copy link
Contributor

Pff.. chicken

.pre-commit-config.yaml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
chia/wallet/wallet_protocol.py Outdated Show resolved Hide resolved
chia/wallet/util/debug_spend_bundle.py Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Oct 25, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 25, 2024
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Oct 25, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 25, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Oct 25, 2024
Copy link
Contributor

File Coverage Missing Lines
chia/_tests/build-init-files.py 0.0% lines 92
chia/_tests/util/misc.py 0.0% lines 351
chia/_tests/wallet/rpc/test_wallet_rpc.py 0.0% lines 284
chia/server/chia_policy.py 50.0% lines 264, 295
chia/util/db_wrapper.py 0.0% lines 117
chia/util/dump_keyring.py 0.0% lines 89
chia/util/task_timing.py 0.0% lines 131, 165
Total Missing Coverage
75 lines 9 lines 88%

@emlowe
Copy link
Contributor

emlowe commented Oct 25, 2024

Coverage diff exemption

@pmaslana pmaslana merged commit 6084112 into main Oct 25, 2024
359 of 360 checks passed
@pmaslana pmaslana deleted the quex.replace_pylint_with_ruff branch October 25, 2024 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added Required label for PR that categorizes merge commit message as "Added" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants