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

Can't use colon in output lines because item parsing is too greedy #155

Closed
mjpieters opened this issue Dec 16, 2024 · 1 comment · Fixed by #157
Closed

Can't use colon in output lines because item parsing is too greedy #155

mjpieters opened this issue Dec 16, 2024 · 1 comment · Fixed by #157

Comments

@mjpieters
Copy link
Contributor

mjpieters commented Dec 16, 2024

The way that out lines are matched is not allowing for colons in the output message, because the severity group can capture up to the last colon in the message:

r"^(?P<fname>.*):(?P<lnum>\d*): (?P<severity>.*):((?P<col>\d+):)? (?P<message>.*)$"

Given a typical input with type annotations in a function:

main:2: note: Revealed type is def(foo: str) -> int

the severity group captures the text note: Revealed type is def(foo.

With more colons, the group ever expands; given

main:2: note: Revealed type is def(foo: str, bar: int, baz: float) -> int

the severity group matches note: Revealed type is def(foo: str, bar: int, baz.

See https://regex101.com/r/wOwuXG/1 for how the groups are filled.

Please match only non-colon text for severity:

(?P<severity>[^:]*)

to prevent this. See https://regex101.com/r/sM2uam/1 for a demo.

@mjpieters
Copy link
Contributor Author

mjpieters commented Dec 20, 2024

Here are two test-cases, that, when added to pytest_mypy_plugins/tests/test-regex-assertions.yml, should both pass:

- case: regex_against_callable_comment
  main: |
    def foo(bar: str, ham: int = 42) -> set[str | int]:
      return {bar, ham}
    reveal_type(foo)  # NR: Revealed type is "def \(bar: builtins\.str, ham: builtins\.int =\) -> .*"

- case: regex_against_callable_out
  regex: yes
  main: |
    def foo(bar: str, ham: int = 42) -> set[str | int]:
      return {bar, ham}
    reveal_type(foo)
  out: |
    main:3: note: Revealed type is "def \(bar: builtins\.str, ham: builtins\.int =\) -> .*"

However, the second case fails even though the regex is the same. Both pass when I replace (?P<severity>.*) with (?P<severity>[^:]+). Arguably the pattern should be tightened further, the severity is always expressed using ASCII letters only, for example.

I'll create a PR for this.

mjpieters added a commit to mjpieters/pytest-mypy-plugins that referenced this issue Dec 20, 2024
The `severity` group was permissive enough to also capture parts of
the message text if that contained any colons. Update it to only capture
(ASCII) words, and update preceding groups to match at least 1
character.

Fixes typeddjango#155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant