Skip to content
This repository was archived by the owner on Oct 30, 2026. It is now read-only.

Reject option-like file paths before invoking Pylint - #126

Open
madmath wants to merge 1 commit into
Shopify:masterfrom
madmath:secfix/secfix-3-pylint-paths
Open

madmath wants to merge 1 commit into
Shopify:masterfrom
madmath:secfix/secfix-3-pylint-paths

Conversation

@madmath

@madmath madmath commented Oct 29, 2026 •

Copy link
Copy Markdown

Keep caller-supplied file paths from being interpreted as linter options.

Compatibility review needed: this rejects leading-dash paths with ValueError rather than normalizing them. Callers relying on those paths may need changes; supported legacy Python versions remain unverified.

Validation used Python 3.11.15, Pylint 2.17.7 and pytest 7.4.4 because the declared legacy environment was unavailable:

  • python -m pytest -q tests/shopify_python/test_git_utils.py::test_linter_rejects_paths_that_look_like_options: fails before the guard, passes after.
  • python -m pytest -q tests/shopify_python/test_git_utils.py --randomly-seed=1: 12 passed, 3 failed; the same 3 existing failures occur on the baseline.
  • make run_tests: 43 passed, 37 failed, 2 skipped; all 37 failures also occur on the baseline.
  • pycodestyle shopify_python/git_utils.py tests/shopify_python/test_git_utils.py and git diff --check: pass.
  • make lint: fails on legacy config/plugin incompatibilities; modern Pylint also requests an f-string for the Python-2-compatible error message. mypy --py2 shopify_python tests/shopify_python --ignore-missing-imports cannot run without typed_ast.

Co-authored-by: River <river@shopify.com>
@madmath
madmath marked this pull request as ready for review October 29, 2026 15:15
@madmath

madmath commented Oct 29, 2026

Copy link
Copy Markdown
Author

@stephanie-shopify could you review this change? Please see the description for compatibility questions and verification limitations.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant