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

Fix missing MatchErrors due to hash collisions #4307

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

cavcrosby
Copy link
Contributor

Fixes #4297.

This fix follows what was done here, #4202, for transitioning to passing a lintable argument for MatchError object construction instead of a filename argument. I also noticed a few places in src/ansiblelint/utils.py where this problem could be reproduced. Hence, I figured now would be a good time to address those spots as well.

Copy link

sonarcloud bot commented Sep 3, 2024

@marcandre-larochelle-bell
Copy link

marcandre-larochelle-bell commented Sep 6, 2024

@cavcrosby

I think the function _hash_key() also require to be updated to take into account the lintable instead of the filename (in MatchError) see:

@property
def _hash_key(self) -> Any:
# line attr is knowingly excluded, as dict is not hashable
return (
self.filename,
self.lineno,
str(getattr(self.rule, "id", 0)),
self.message,
self.details,
# -1 is used here to force errors with no column to sort before
# all other errors.
-1 if self.column is None else self.column,
)

Setting the filename for match_error has been removed because this will
be done on MatchError instance creation via the __post_init__ method.
@ssbarnea ssbarnea merged commit 712b530 into ansible:main Sep 19, 2024
26 checks passed
@cavcrosby cavcrosby deleted the fix-matcherrors-hash-collision branch September 21, 2024 13:39
@cavcrosby
Copy link
Contributor Author

@marcandre-larochelle-bell I apologize for not getting around to your comment until now. I think it's fine to continue using the filename attribute in the _hash_key method at this time.

@marcandre-larochelle-bell
Copy link

marcandre-larochelle-bell commented Sep 23, 2024

@cavcrosby No worries, I was mostly wondering, thanks for the reply!

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

Successfully merging this pull request may close these issues.

Missing Rule breaks due to hash collisions for dataclass MatchError
3 participants