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

Missing file name when emiting warnings #5729

Closed
matejsp opened this issue Jul 13, 2023 · 5 comments · Fixed by #5856
Closed

Missing file name when emiting warnings #5729

matejsp opened this issue Jul 13, 2023 · 5 comments · Fixed by #5856

Comments

@matejsp
Copy link

matejsp commented Jul 13, 2023

(venv) ➜  rm -rf .ruff_cache && ruff .
warning: Invalid `# noqa` directive on line 6: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 1: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 7: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 3: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
warning: Invalid `# noqa` directive on line 25: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).

This warning is emitted only on first run (after cache is build it is no longer displayed)
It would be nice to threat warnings as errors

Ruff version:
(venv) ➜ ruff --version
ruff 0.0.27

Previous version didn't emit such warnings on lines:
# noqa: 901
now it wants complete code:
# noqa: C901

@zanieb
Copy link
Member

zanieb commented Jul 13, 2023

@matejsp just to clarify, you're looking for three things here?

  1. Include the file name in the warnings
  2. Always emit warnings, even if the cache exists
  3. The ability to error on warnings

Is that right?

@matejsp
Copy link
Author

matejsp commented Jul 13, 2023

  1. YES (bug title)
  2. would be nice
  3. would also be nice :D

@charliermarsh
Copy link
Member

I think the "right" solution here might be to treat those warnings as diagnostics, rather than "warnings". As diagnostics, they would have all of those properties automatically.

@charliermarsh
Copy link
Member

It's similar to #850.

@charliermarsh
Copy link
Member

Maybe I'll rephrase that as: they should very likely be diagnostics, and not ad hoc warnings. But the downside of being diagnostics is that they won't be enabled by default for users.

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

Successfully merging a pull request may close this issue.

3 participants