Skip to content

Add noqa skips in otherwise empty lines to the next non-empty line #4567

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

StopMotionCuber
Copy link
Contributor

This addresses #3935

So what it does:

  • Some magic in traverse_yaml to flatten lists within there (these are required for a ruamel CommentMap, please don't ask any further, afaik these are entirely undocumented)
  • After collecting all #noqa and inserting them into the lintable, postprocessing is done on the lintable.
    • For each line where a skip is found, it is matched against a regex whether # noqa is the start of the line, minus some whitespace/indentation.
    • If there is only the # noqa on that line, the skip is added to the next non-empty line.

This is something required for me in order to suppress errors on yaml multiline strings, as reported by the issue referenced.

In general I think the solution is quite hacky, so this can also be seen as a proof of concept. The whole flattening thing and how ruamel works with comments is quite weird. As in the context of _get_rule_skips_from_yaml only a map of {lineno: rule_to_skip} is what we're interested in, it might be easier to just go over the content of a file, check each line for _noqa_comment_re match. The whole tree structure of CommentMap is annoying to traverse, a simple iterator over all comments would do it (don't know if that's in pyyaml?) and would be more accurate for comments not attached to objects in a line.

@ssbarnea
Copy link
Member

ssbarnea commented Apr 1, 2025

This will have to wait until we finish the changes started in #4564 - adding support for ansible 2.19 with data tagging. There were a really big number of changes related to this and they also affect this area.

@StopMotionCuber
Copy link
Contributor Author

That makes sense. I already noticed that PR. I didn't see any commit to skip_utils.py so I thought it was fine to go ahead, and I couldn't read the JIRA Ticket to get more information 😕

I'd happily contribute as soon as refactoring is done, just ping me again

@StopMotionCuber
Copy link
Contributor Author

@ssbarnea I guess the data tagging changes are integrated now, any chance you could take a look again?

@alisonlhart alisonlhart requested a review from ssbarnea April 23, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants