Pragma comment handling #6670
Replies: 5 comments 4 replies
-
|
@roshanjrajan-zip I would love your reaction to this proposal since you expressed that you ran into issues with Black's pragma comment handling. |
Beta Was this translation helpful? Give feedback.
-
|
Not only is this an amazing overview, I happen to agree with you entirely and approve this proposal 🥳 |
Beta Was this translation helpful? Give feedback.
-
|
main concern: how do we teach this? i.e. how would a user no why ruff suddenly decided to bail on enforcing the line length limit on this specific line? |
Beta Was this translation helpful? Give feedback.
-
|
This is such a good write-up. It's really impressive. I like the proposal a lot.
I believe that your description is correct. (There is at least one deviation between Ruff and Flake8: Ruff requires that a Separately: are there any lessons we should takeaway for any suppression comment redesigns we explore in the future? (For Ruff's suppression comments, at least.) |
Beta Was this translation helpful? Give feedback.
-
|
There is also |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Formatting Pragma Comments
Various static analysis tools use pragma comments to attribute code. One such pragma comment is Ruff's
noqacomment. Formatting pragma comments faces the same challenge as regular comments: There's no grammar rule specifying what a comment comments. It is only possible to rely on heuristics. While good heuristics are important for regular comments to preserve their context, pragma comments also have semantical meaning that may change when a comment is moved. The main challenges when it comes to placing pragma comments are:What they apply to depends on the comment's semantics
Whether
noqa: XXXsuppresses a violation ofaorcall()depends on the details of the ruleXXX. The only safe (and reasonably fast) assumption the formatter can make is that the pragma comment suppresses all code on the same line. Type hint commentstype: Tandnocoveragecomments have different semantics: they annotate the preceding statement or clause.There's the tension between: 1) precisely preserving the pragma comment's semantics, and 2) formatting the code and respecting the preferred line width. Perfectly solving for 1) means that the formatter needs to preserve the line breaks of affected lines, meaning they unnecessarily split over multiple lines or exceed the configured line width. Perfectly solving for 2) means that pragma comments may move and change (or lose) their semantics.
Principles
Suppression Range
Prefer reducing the range of pragma comments rather than extending them because tools report missing suppression. Some tools even support adding and removing unnecessary suppressions (including Ruff). Extended suppression ranges go undetected and can suppress real issues in the future that may lead to production incidents (The very thing our users try to avoid by using our static analysis tooling).
The way to achieve this is by eagerly splitting if a node has a trailing (suppression comment). This requires more vertical space than collapsing the logical line if it fits into the line width. This also prevents multiple suppression comments from collapsing, which not all tools support. The following
type: ignorecomment no longer applies because a# noqacomment precedes it.Black handles
type:comments to prevent this specific behavior. However, it may be a problem for other pragma comments.Another way to phrase this principle is that we want to respect the user's comment placements. The user deliberately added the
noqacomment afterb. We should respect that decision and preserve it.Formatting
Favor manual comment placement over disabling or limiting formatting.
People use formatters to avoid discussing formatting decisions. Disabling formatting, or reduced formatting resurfaces the very same problem that people try to avoid by using a formatter. This isn't the same as us not caring about pragma comments or not wanting to use any pragma comment specific formatting. It only means that we don't want to use solutions that disable or limit formatting in a significant way.
Proposal
# documentation # noqa:fully count towards the line width, but# noqa: XXX # documentationwill not, because it would require the formatter to understand the semantics.Alternatives
type:comments similar to Black [PR]. I understand its motivation but believe that limiting formatting for entire lines removes the advantage of using a formatter. Not counting pragma comments towards the line width balances "reducing the frustration when placing pragma comments" with "the benefits of using a formatter" better IMO. I admit that this approach doesn't solve the issue when you format a new project or extend the width of an existing line. But moving the comment manually is a reasonable ask in these situations.Examples
Ruff's default behavior is to keep end-of-line comments on the same line as the node they annotate. In-between operator comments have different formatting, and Ruff can move them to another line.
Case 1: Suppress single argument/expression
Ruff's philosophy on trailing same-line comments is to keep them close to the node they annotate. This prevents increasing the scope of pragma comments.
Black's formatting has the disadvantage that it extends the suppression from
bto all arguments. This can be worked around by manually adding a magic trailing comma and moving the comment back. But this requires more manual work and may go unnoticed when formatting the whole project.Black issue
Case 2: Breaking overlong lines
Both formatters move the
noqasuppression to the end. The result is that thenoqacomment no longer suppresses any violations because it only applies to):.Black special cases
type: ignoreat the end of physical lines to prevent these lines from splitting. Although, this doesn't seem to work for case 3 or the following example.I believe it is because Black only tests for the pragma comment at the end of logical lines, the end after having split the line. But that means you must know Black's internal formatting to understand in which positions Black preserves
type: ignorecomments.Moving the
noqacomment past the):is admittedly the worst position to move the suppression comment because it never suppresses anything (may not be true for other pragma comments). It's difficult to place the comment better for function headers, but the comment could be moved to a more meaningful position as proposed in this Black issue commentThis is only a heuristic, and we may need to exclude
type: ignorecomments because they must come at the statement's end.Case 3: Overlong with multiple items
Both formatters expand the arguments and keep the
noqaattached to the secondignored()call and break the binary expression over multiple lines, excludingignoredfrom the suppression.Case 4: Suppression after open parentheses
Both formatters preserve the comment position.
Case 5: Ruff Code fix
Edge case of using
--add-noqawith the formatter:We can avoid this (and similar experiences where users manually add noqa comments) by not accounting noqa comments to the line width. Related Dartfmt issue. They decided against it because they can't feasibly implement this in their formatter.
Another solution would be that ruff tries to apply the noqa comment to the smallest range necessary and make it split lines where allowed. Unfortunately, this isn't an easy task because inserting a line break may a) not be allowed or b) require parenthesizing the outer expression.
Other Tools
Handling of line-based pragma comments is something all formatters I know struggle with. Rust doesn't have this problem because its suppression comments are node directed, allowing the formatter to reason about their semantical meaning.
typecomments.Pragma comments in the Python ecosystem
Type comments
type: THave the form
type: Tand were introduced by PEP484. They annotate the type of a variable.Type comments should be put on the statement's last line containing the variable definition. They can also be placed on
withstatements andforstatements, right after the colon.Type comments are specific to assignment,
with, andforstatements.Type information are available at runtime for introspection. Changing the placement of any of these comments may change the runtime semantics of a program.
Multiline annotations
This seems to be the correct annotation according to pycharm
the following doesn't provide the right type-hint:
type: ignoreThe PEP484 further introduces
type: ignorecomments.They have to be placed at the end of a physical line.
# type: ignore # <comment or other marker>Pyright
File level comments
Own-line comments that control the strictness of Pyright. These shouldn't be a problem because they must appear on their own line. The documentation isn't specific about where file-level comments are valid but assuming they are only used in-between statements (e.g. not between expressions) seems reasonable.
Line-Level Suppression comments
Pyright supports
type: ignoreandpyright: ignore. Pyright ignore supports suppressing specific errors.Noqa
Noqa comments are special because Ruff uses them itself.
The flake8 documentation isn't very specific about their semantics. That's why I rely on Charlie's understanding.
Pylint
Coverage.py
Clause-based ignore comments are unproblematic because we preserve them.
Resources
Related issues:
Relevant Black code and here
Beta Was this translation helpful? Give feedback.
All reactions