Fix crash when an attribute node was used inside an unary op

[Pierre-Sassoulas]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Closes #8207

[clavedeluna]

I was just writing a PR to fix this since I feel responsible for writing it! The real fix turns out to be

    def _get_start_value(self, node: nodes.NodeNG) -> tuple[int | None, Confidence]:
        inferred = utils.safe_infer(node)
        start_val = inferred.value if inferred else None
        return start_val, INFERENCE

when I initially wrote it I was just not as familiar with when to use inference so I was super conservative, but turns out this should handle all cases!

[clavedeluna]

You'll also need to update the output .txt with my suggestion

[Pierre-Sassoulas]

Isn't it better to not use inference all the time for performance ?

[codecov]

Codecov Report

Merging #8209 (d933d3f) into main (7612939) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8209   +/-   ##
=======================================
  Coverage   95.45%   95.45%           
=======================================
  Files         177      177           
  Lines       18635    18635           
=======================================
  Hits        17788    17788           
  Misses        847      847           

Impacted Files	Coverage Δ
pylint/checkers/refactoring/refactoring_checker.py	98.33% <100.00%> (ø)
pylint/checkers/base/basic_error_checker.py	95.51% <0.00%> (ø)

[clavedeluna]

Isn't it better to not use inference all the time for performance ?

You'd know better than I :) but I guess here we're trading performance for preventing another one of these bugs in the future in case there is another type of node we didn't think of in the tests.

[Pierre-Sassoulas]

I think at this point we pretty much covered all the crash cases, the issues are getting increasingly niche. But yeah it crashed a lot since initial implementation. 😄 What do you think ?

You'll also need to update the output .txt with my suggestion

I did not understand that part ?

[clavedeluna]

I think at this point we pretty much covered all the crash cases, the issues are getting increasingly niche. But yeah it crashed a lot since initial implementation. 😄 What do you think ?

You'll also need to update the output .txt with my suggestion

I did not understand that part ?

No need to update anything since we've discussed it.

[Pierre-Sassoulas]

I have no idea if the performance are better with the current code compared to inference tbh. And what you proposed looks a lot better and also fail proof.

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit d933d3f

[Pierre-Sassoulas]

@DanielNoord do you have an opinion on the subject ?

[DanielNoord]

This looks fine to me! I don't really unferstand the suggestion by @clavedeluna as .values still isn't safe, but this follows the pattern we normally use!