Conversation
github-actions
bot
added
src
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the initial release of noqaexplain, a linter that enforces explanations for all ignored linting rules across multiple languages and linters.
Key Changes
- Implements core linting functionality with two rules (ENQ0 and ENQ1) to check for missing or too-short explanations
- Adds support for multiple languages/linters: Python (ruff, flake8), JavaScript/TypeScript (eslint), Rust (clippy), Dockerfiles (hadolint), YAML (yamllint), and Shell (shellcheck)
- Provides CLI interface with configuration support via pyproject.toml or .noqaexplain.toml
Reviewed Changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/noqaexplain/init.py | Updated package docstring |
| src/noqaexplain/_cli.py | CLI entrypoint implementation with file discovery and configuration loading |
| src/noqaexplain/_default.py | Default mappings for file extensions to noqa patterns |
| src/noqaexplain/_match.py | File and line pattern matching logic for noqa comments |
| src/noqaexplain/_rule.py | Rule definitions for NoExplain (ENQ0) and NoExplainShort (ENQ1) |
| tests/test_smoke.py | Added smoke test for rules command |
| tests/test_rules.py | Comprehensive CLI tests for pass/fail cases |
| tests/cases/* | Test fixtures for various pass/fail scenarios |
| README.md | Complete documentation with usage, configuration, and examples |
| .pre-commit-hooks.yaml | Pre-commit hook configuration for noqaexplain |
| pyproject.toml | Added lintkit and loadfig dependencies, CLI script entry point |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for extension, noqas in self.config.get( | ||
| f"extend_{name}_mapping", {} | ||
| ).items(): | ||
| mapping[extension].extend(noqas) # pragma: no cover |
There was a problem hiding this comment.
The default mapping comment has a typo. "noqas" should be "noqa" for consistency with the terminology used throughout the project.
| for extension, noqas in self.config.get( | |
| f"extend_{name}_mapping", {} | |
| ).items(): | |
| mapping[extension].extend(noqas) # pragma: no cover | |
| for extension, noqa in self.config.get( | |
| f"extend_{name}_mapping", {} | |
| ).items(): | |
| mapping[extension].extend(noqa) # pragma: no cover |
|
|
||
| lintkit.settings.name = "ENQ" | ||
|
|
||
| # enoqa: Import all rules to register them (side-effect) |
There was a problem hiding this comment.
There's a spelling error in the comment. "enq" should be "# enq:" (with the # prefix and colon) to be clearer about the format, or just say "explanation" to be more general. The current wording is ambiguous.
| # enoqa: Import all rules to register them (side-effect) | |
| # enq: Import all rules to register them (side-effect) |
|
|
||
| ... | ||
| ```sh | ||
| > noqaxplain check |
There was a problem hiding this comment.
There's a typo in the shell command example. "noqaxplain" should be "noqaexplain".
| > noqaxplain check | |
| > noqaexplain check |
| for example: | ||
|
|
||
| ```toml | ||
| [tool.noqexplain] |
There was a problem hiding this comment.
There's a typo in the TOML section header. "noqexplain" should be "noqaexplain".
| [tool.noqexplain] | |
| [tool.noqaexplain] |
|
|
||
| ### Configuration | ||
|
|
||
| You can configure pynudger in `pyproject.toml` (or `.noqaexplain.toml` |
There was a problem hiding this comment.
Incorrect reference in the comment. It says "pynudger" but should be "noqaexplain" since this is the noqaexplain project configuration.
| You can configure pynudger in `pyproject.toml` (or `.noqaexplain.toml` | |
| You can configure noqaexplain in `pyproject.toml` (or `.noqaexplain.toml` |
| def file(self, path: pathlib.Path) -> list[str]: | ||
| """Match `path` against suffixes and names. | ||
|
|
||
| Warning: | ||
| Name patterns have higher priority and might override | ||
| suffix patterns. | ||
|
|
||
| Args: | ||
| path: | ||
| Path to be matched. | ||
|
|
||
| Returns: | ||
| List (possibly empty) of matching patterns. | ||
|
|
||
| """ | ||
| suffix_patterns = self.suffix_mapping.get(path.suffix, []) |
There was a problem hiding this comment.
[nitpick] Inconsistent terminology: the docstring says "noqa" but the actual check pattern could be extended to more than just "noqa" patterns. Consider using "linting rule disable patterns" or similar more general terminology, or clarify that "noqa" is used generically here.
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Module to match file and its line within file against noqa pattern(s).""" |
There was a problem hiding this comment.
There's a typo in the comment. "noqas" should be "noqa" (singular) to match the correct terminology used elsewhere in the codebase.
| from noqaexplain import ( # noqa: E402 | ||
| _rule, # noqa: F401 # pyright: ignore[reportUnusedImport] | ||
| ) |
There was a problem hiding this comment.
Import of '_rule' is not used.
Signed-off-by: szymonmaszke <github@maszke.co>
1 participant
Checklist