Skip to content

feat(version-scanner): implement targeted namespaced ignore pragmas#17540

Open
chalmerlowe wants to merge 1 commit into
feat/version-scanner-pr2from
feat/version-scanner-pr3
Open

feat(version-scanner): implement targeted namespaced ignore pragmas#17540
chalmerlowe wants to merge 1 commit into
feat/version-scanner-pr2from
feat/version-scanner-pr3

Conversation

@chalmerlowe

@chalmerlowe chalmerlowe commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

This PR implements ignore pragmas to allow granular and self-expiring ignores to help fend off false positives in places where we need to leave references to specific strings:

# version-scanner: ignore-rule=rule_name:version

Example:

Imagine searching for "Python 3.7". We have a variety of rules to cover multiple circumstances. One rule matches solely on the version number (e.g. "3.7" in case the reference is an edge case not covered by other more specific and complicated rules).

Thus if we find "3.7" in a situation unrelated to Python, we will have a false positive such as this:
matplotlib==3.7.2

We can flag a line like this to be ignored under one of the numerous rules we have for categorizing matches (e.g explicit_version_string, dependency_requirement, combined_version_string, etc.) so that it does not trigger a false positive in the future. In this case we detected it under the explicit_version_string pattern so we flag it to be ignored under that rule, if the search is for 3.7.

matplotlib==3.7.2 # version-scanner: ignore-rule=explicit_version_string:3.7

You might ask: but what about when we need to find versions of matplotlib so we can update references to them?

If instead of Python, we were to search for "matplotlib 3.7.2" we would get a match. As a non-Python match, it would be caught under the dependency_requirement rule instead of the explicit_* rule.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for targeted rule ignoring in the version scanner using the version-scanner: ignore-rule=<rule_name>:<version> pragma, along with corresponding unit tests. Feedback was provided to convert the version value to a string before calling re.escape to avoid potential runtime TypeError exceptions if a non-string version (such as a float or integer) is specified in the rule configuration.

Comment thread scripts/version_scanner/version_scanner.py
@chalmerlowe chalmerlowe marked this pull request as ready for review June 23, 2026 15:07
@chalmerlowe chalmerlowe requested a review from a team as a code owner June 23, 2026 15:07
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr2 branch from 04f3d2d to 69e81ab Compare June 23, 2026 15:21
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr3 branch from 64367eb to 23b8ef3 Compare June 23, 2026 15:21
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr2 branch from 69e81ab to 111c152 Compare June 23, 2026 15:31
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr3 branch from 23b8ef3 to 031be67 Compare June 23, 2026 15:31
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr2 branch from 111c152 to 055975c Compare June 23, 2026 15:57
@chalmerlowe chalmerlowe force-pushed the feat/version-scanner-pr3 branch from 031be67 to 8abd504 Compare June 23, 2026 15:58
@chalmerlowe chalmerlowe marked this pull request as draft June 23, 2026 16:39
@chalmerlowe chalmerlowe marked this pull request as ready for review June 24, 2026 16:33
@chalmerlowe chalmerlowe marked this pull request as draft June 24, 2026 16:34
@chalmerlowe chalmerlowe added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 24, 2026
@chalmerlowe chalmerlowe marked this pull request as ready for review June 24, 2026 16:34
@chalmerlowe

Copy link
Copy Markdown
Contributor Author

This is marked "DO NOT MERGE" simply because it is blocked by a precursor PR. It is fine to review this PR and approve, but it should not be merged until that other PR is merged.

@hebaalazzeh hebaalazzeh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! just one suggestion to cast the rule name and version to strings before escaping them. This will prevent potential TypeError exceptions if a version (like 3.7) is ever parsed from the config as a float instead of a string.

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

Labels

do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants