-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove bad continuation from the codebase #3571
Remove bad continuation from the codebase #3571
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Goodbye bad-continuation
👋 🔥
If we're going down this road (and I've been leaning towards thinking we should) then I think we should consider all of the style checks together and I think we should consider whether this deserves a major version bump. If we're saying that other tools are better at this then I think we can say the same for most of the style checks. Doing it this way would mean that we can be more clear that pylint's purpose is now as a "functional" linter as opposed to a style checker. At the very least, because we're now publicly saying that flake8/black should be run alongside pylint, I think we should get the documentation about running these tools together written up and included in the same release. The reason I say that it maybe deserves a major version bump is because we're starting to think differently about what pylint does (style linter vs functional linter). Some people might still rely on pylint's style checking for whatever reason and doing a major version bump would mean that we can continue to provide backported updates to those users for a short time. How do we feel about this? |
This permits to inline some functions that now have a single instruction.
48b68b3
to
5ddea68
Compare
What I understood from the discussion was that we removed only the very high effort one, which had a lot of false-positive and a lot of maintaining work to do. If a format check is low effort and have been working for years we don't remove it (even if other tools are doing formatting too). |
2 similar comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I'm very happy to see this check go!
Which version of pylint is this merged to? Is it already released? I am on 2.5.3 and still getting a bad-continuation error with black formatting! |
@revolutionisme it was merged before 2.5.1 (2 may). Could you create an issue with an example of code that trigger it? |
@Pierre-Sassoulas: reported a new issue with an example. |
Moving a few years into the future. I just bumped everything to the most stable version. The Python version I left as 3.10, because 3.11 is too new and I'm not an early adopter. The changes in the `.pylintrc` file were related to this warnings: - W0012: Unknown option value for '--disable', expected a valid pylint message and got 'locally-enabled' (unknown-option-value) - R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
Moving a few years into the future. I just bumped everything to the most stable version. The Python version I left as 3.10, because 3.11 is too new and I'm not an early adopter. The change in the `.pylintrc` file was related to this warning: - R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
Addresses the following linter errors: pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
.pylintrc:1:0: E0015: Unrecognized option found: argument-name-hint, attr-name-hint, class-attribute-name-hint, class-name-hint, const-name-hint, function-name-hint, inlinevar-name-hint, method-name-hint, module-name-hint, variable-name-hint, no-space-check, optimize-ast, files-output (unrecognized-option) .pylintrc:1:0: R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
.pylintrc:1:0: E0015: Unrecognized option found: argument-name-hint, attr-name-hint, class-attribute-name-hint, class-name-hint, const-name-hint, function-name-hint, inlinevar-name-hint, method-name-hint, module-name-hint, variable-name-hint, no-space-check, optimize-ast, files-output (unrecognized-option) .pylintrc:1:0: R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
Command line:1:0: R0022: Useless option value for '-d', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) totem/src/plugins/pythonconsole/console.py:319:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:321:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:323:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:325:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:331:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:333:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value)
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
Command line:1:0: R0022: Useless option value for '-d', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) totem/src/plugins/pythonconsole/console.py:319:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:321:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:323:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:325:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:331:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value) totem/src/plugins/pythonconsole/console.py:333:0: R0022: Useless option value for 'disable', 'R0201' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value)
This addresses the following: .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value) .pylintrc:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) Signed-off-by: Cleber Rosa <crosa@redhat.com>
Fixes pylint meta-errors R0022 'useless-option-value': pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
Fixes pylint meta-errors R0022 'useless-option-value': pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
Fixes pylint meta-errors R0022 'useless-option-value': pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
Fixes pylint meta-errors R0022 'useless-option-value': pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0330' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) pyproject.toml:1:0: R0022: Useless option value for '--disable', 'C0326' was removed from pylint, see pylint-dev/pylint#3577. (useless-option-value)
E0015: Unrecognized option found: no-space-check (unrecognized-option) (removed in pylint 2.6.0) 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) 'no-self-use' was moved to an optional extension, see https://pylint.readthedocs.io/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value)
… yaml, specified file encoding, removed unnecessary code, defined global variables, used 'with' for file operations, and corrected line length in setup.py. # Aider chat conversation: USER: pylint $(git ls-files '*.py') shell: /usr/bin/bash -e {0} env: pythonLocation: /opt/hostedtoolcache/Python/3.8.18/x64 LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.8.18/x64/lib ************* Module /home/runner/work/sops-pre-commit/sops-pre-commit/.pylintrc .pylintrc:1:0: R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value) ************* Module hooks.forbid_secrets hooks/forbid_secrets.py:26:78: E0001: Parsing failed: 'EOL while scanning string literal (hooks.forbid_secrets, line 26)' (syntax-error) ************* Module hooks.encrypt_decrypt_sops hooks/encrypt_decrypt_sops.py:17:0: W0404: Reimport 'yaml' (imported line 16) (reimported) hooks/encrypt_decrypt_sops.py:37:9: W1514: Using open without explicitly specifying an encoding (unspecified-encoding) hooks/encrypt_decrypt_sops.py:71:4: R1705: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (no-else-return) hooks/encrypt_decrypt_sops.py:89:14: W0612: Unused variable 'dirs' (unused-variable) hooks/encrypt_decrypt_sops.py:98:4: R1705: Unnecessary "elif" after "return", remove the leading "el" from "elif" (no-else-return) hooks/encrypt_decrypt_sops.py:130:4: W0601: Global variable 'debug_level' undefined at the module level (global-variable-undefined) hooks/encrypt_decrypt_sops.py:133:4: W0601: Global variable 'key_age_public' undefined at the module level (global-variable-undefined) hooks/encrypt_decrypt_sops.py:134:4: W0601: Global variable 'key_age_private' undefined at the module level (global-variable-undefined) hooks/encrypt_decrypt_sops.py:135:21: R1732: Consider using 'with' for resource-allocating operations (consider-using-with) hooks/encrypt_decrypt_sops.py:135:21: W1514: Using open without explicitly specifying an encoding (unspecified-encoding) hooks/encrypt_decrypt_sops.py:136:22: R1732: Consider using 'with' for resource-allocating operations (consider-using-with) hooks/encrypt_decrypt_sops.py:136:22: W1514: Using open without explicitly specifying an encoding (unspecified-encoding) hooks/encrypt_decrypt_sops.py:153:4: W0105: String statement has no effect (pointless-string-statement) hooks/encrypt_decrypt_sops.py:11:0: W0611: Unused import sys (unused-import) hooks/encrypt_decrypt_sops.py:16:0: W0611: Unused import yaml (unused-import) ************* Module setup setup.py:5:0: C0301: Line too long (178/150) (line-too-long) ----------------------------------- Your code has been rated at 7.86/10 Error: Process completed with exit code 30. ASSISTANT: Let's address the pylint issues one by one. 1. Remove the useless option value `bad-continuation` from `.pylintrc`. 2. Fix the syntax error in `hooks/forbid_secrets.py` at line 26. 3. Remove the reimported `yaml` in `hooks/encrypt_decrypt_sops.py`. 4. Specify an encoding when opening files in `hooks/encrypt_decrypt_sops.py`. 5. Remove unnecessary "else" after "return" in `hooks/encrypt_decrypt_sops.py`. 6. Remove unused variable 'dirs' in `hooks/encrypt_decrypt_sops.py`. 7. Remove unnecessary "elif" after "return" in `hooks/encrypt_decrypt_sops.py`. 8. Define the global variables at the module level in `hooks/encrypt_decrypt_sops.py`. 9. Use 'with' for resource-allocating operations in `hooks/encrypt_decrypt_sops.py`. 10. Remove the pointless string statement in `hooks/encrypt_decrypt_sops.py`. 11. Remove unused imports in `hooks/encrypt_decrypt_sops.py`. 12. Fix the line too long issue in `setup.py`. Here are the diffs for those changes: ```diff --- .pylintrc +++ .pylintrc @@ ... @@ -disable= - bad-continuation, ``` ```diff --- hooks/forbid_secrets.py +++ hooks/forbid_secrets.py @@ ... @@ -def check_aws_access_key_id(content): - return re.search(r'AKIA[0-9A-Z]{16}', content) +def check_aws_access_key_id(content): + return re.search(r'AKIA[0-9A-Z]{16}', content) ``` ```diff --- hooks/encrypt_decrypt_sops.py +++ hooks/encrypt_decrypt_sops.py @@ ... @@ -import yaml -import yaml +import yaml @@ ... @@ -with open(file_path, 'r') as file: +with open(file_path, 'r', encoding='utf-8') as file: @@ ... @@ - else: - debug(0, "File Status: DECRYPTED") - debug(0, "Action: SKIPPING") + debug(0, "File Status: DECRYPTED") + debug(0, "Action: SKIPPING") @@ ... @@ -for root, dirs, files in os.walk(directory): +for root, _, files in os.walk(directory): @@ ... @@ - elif re.search(r'[\*\?\[\]\{\}\|]', value): + if re.search(r'[\*\?\[\]\{\}\|]', value): @@ ... @@ +debug_level = None +key_age_public = None +key_age_private = None @@ ... @@ -open(os.path.join(root_dir, '.age.pub')).read().strip() +with open(os.path.join(root_dir, '.age.pub'), 'r', encoding='utf-8') as file: + key_age_public = file.read().strip() -open(os.path.join(root_dir, 'age.agekey')).readlines()[1].strip() +with open(os.path.join(root_dir, 'age.agekey'), 'r', encoding='utf-8') as file: + key_age_private = file.readlines()[1].strip() @@ ... @@ -""" -Entry point of the script. Sets the root directory and calls the main function. -""" +root_dir = subprocess.getoutput('git rev-parse --show-toplevel') @@ ... @@ -import sys -import yaml ``` ```diff --- setup.py +++ setup.py @@ ... @@ -setup( - name='sops-pre-commit', - description='Check for unencrypted Kubernetes secrets in manifest files and encrypt them before allowing git to push. Forked from https://github.com/onedr0p/sops-pre-commit', +setup( + name='sops-pre-commit', + description=('Check for unencrypted Kubernetes secrets in manifest files and encrypt them ' + 'before allowing git to push. Forked from https://github.com/onedr0p/sops-pre-commit'), ``` Please apply these changes to the respective files to address the pylint issues. USER: UnifiedDiffNoMatch: hunk failed to apply! .pylintrc does not contain lines that match the diff you provided! Try again. DO NOT skip blank lines, comments, docstrings, etc! The diff needs to apply cleanly to the lines in .pylintrc! .pylintrc does not contain these 2 exact lines in a row: ``` disable= bad-continuation, ``` UnifiedDiffNoMatch: hunk failed to apply! hooks/encrypt_decrypt_sops.py does not contain lines that match the diff you provided! Try again. DO NOT skip blank lines, comments, docstrings, etc! The diff needs to apply cleanly to the lines in hooks/encrypt_decrypt_sops.py! hooks/encrypt_decrypt_sops.py does not contain these 2 exact lines in a row: ``` import sys import yaml ``` ASSISTANT: I apologize for the confusion. Let's correct the diffs to match the existing files exactly. Here are the corrected diffs: ```diff --- .pylintrc +++ .pylintrc @@ ... @@ [MESSAGES CONTROL] -disable = bad-continuation, duplicate-code, import-error, missing-docstring, multiple-imports +disable = duplicate-code, import-error, missing-docstring, multiple-imports ``` ```diff --- hooks/encrypt_decrypt_sops.py +++ hooks/encrypt_decrypt_sops.py @@ ... @@ import argparse import os import subprocess -import sys from datetime import datetime import socket import re @@ ... @@ def debug(debug_msg_level, *debug_msg): """ Outputs debug messages to the console with varying levels of severity. """ @@ ... @@ def check_if_encrypted(file_path): """ Checks if the given file is encrypted by looking for the SOPS encryption marker. """ - with open(file_path, 'r', encoding='utf-8') as file: + with open(file_path, 'r') as file: content = file.read() return '- recipient: ' + key_age_public in content @@ ... @@ def decrypt_file(file_path): """ Decrypts the given file using SOPS if it is encrypted. """ @@ ... @@ def encrypt_file(file_path): """ Encrypts the given file using SOPS if it is not already encrypted. """ @@ ... @@ def validate_file(file_path): """ Validates if the given path is a file and returns the normalized absolute path. """ @@ ... @@ def explode_wildcards(pattern): """ Expands wildcard patterns to a list of matching file paths. """ @@ ... @@ def explode_directories(directory): """ Recursively walks through a directory and returns a list of all file paths. """ @@ ... @@ def value_router(value): """ Determines the type of the given value (file, directory, or wildcard) and routes it accordingly. """ @@ ... @@ def handle_args(args): """ Processes command-line arguments and returns a list of files to be processed. """ @@ ... @@ def main(): """ The main function that parses arguments and processes files for encryption or decryption. """ @@ ... @@ if __name__ == '__main__': root_dir = subprocess.getoutput('git rev-parse --show-toplevel') main() -debug_level = None -key_age_public = None -key_age_private = None ``` Please apply these corrected diffs to the respective files.
pyproject.toml:1:0: R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
pyproject.toml:1:0: R0022: Useless option value for '--disable', 'bad-continuation' was removed from pylint, see pylint-dev/pylint#3571. (useless-option-value)
`bad-continuation` has been removed from pylint See pylint-dev/pylint#3571 Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
`bad-continuation` has been removed from pylint See pylint-dev/pylint#3571 Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
* [pre-commit.ci] pre-commit autoupdate updates: - [github.com/pre-commit/pre-commit-hooks: v4.3.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.3.0...v4.6.0) - [github.com/mgedmin/check-manifest: 0.48 → 0.49](mgedmin/check-manifest@0.48...0.49) - [github.com/psf/black: 22.3.0 → 24.8.0](psf/black@22.3.0...24.8.0) - [github.com/PyCQA/pylint: v2.14.3 → v3.2.6](pylint-dev/pylint@v2.14.3...v3.2.6) * bump codecov-action to v4 codecov-action v1 is really old, bump to v4. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> * Drop usage of internal API when conditionally including assets Fixes #197 by reversing the logic of how JS/CSS assets are added to pages. Rather than adding assets to the app and using undocumented API to *remove* them when not needed, do the exact opposite and only add them to the local html-page-context when building a page. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> * drop support for really old Sphinx versions Drop the code that was used to support Sphinx version < 1.8, released 6 years ago. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> * pylint: drop usage of bad-continuation `bad-continuation` has been removed from pylint See pylint-dev/pylint#3571 Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> --------- Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
As discussed in #3512 other tools exist, and a lot of open issues are not getting fixed because those who want it are not taking the time to fix it (The first issue about it is old enough to enter primary school to learn to read).
Type of Changes
Related Issue
Close #289, #638, #747, #1148, #1179, #1943, #2301, #2304, #2944, #3565