[fix] AttributeError crash when a slice is used as a class decorator

[Pierre-Sassoulas]

Type of Changes

	Type
✓	🐛 Bug fix

Description

This was partially implemented by an AI bot (codegen-bot), credited as co-author in the commit. I've done some chores (moving functional tests, fixing the new test, creating the changelog, checking that the changes are actually required and that the not required changes are not actually another crash), and some I did not (creating branches, it could have opened the PR if I wasn't working on a fork, creating the regression tests, making the initial fix). Tried to kept the history truthful to what happened. Overall not sure if it's more work or less work than just copy pasting the regression code and fixing myself (probably more), but it certainly feel new and exciting.

Suggestions on what to try next with this AI bot are welcome, I started with "noqa" handling but it seems a little too complicated. I probably should use this on task with less finesse and more grunt work, like refactor (?)

Closes #10334

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.90%. Comparing base (8a733fc) to head (5b30d91).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10350   +/-   ##
=======================================
  Coverage   95.90%   95.90%           
=======================================
  Files         176      176           
  Lines       19122    19122           
=======================================
  Hits        18339    18339           
  Misses        783      783           

Files with missing lines	Coverage Δ
pylint/checkers/deprecated.py	99.06% <100.00%> (ø)
pylint/checkers/utils.py	95.92% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull Request Overview

This pull request fixes an AttributeError crash that occurred when a slice object was used as a class decorator. The changes include the addition of regression tests to reproduce the issue and adjustments in the Pylint checkers to prevent incorrect attribute access during inference.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File	Description
tests/functional/r/regression_02/regression_10334.txt	Adds expected test output indicating non-callable error.
tests/functional/r/regression_02/regression_10334.py	Provides a test case using a slice as a decorator.
pylint/checkers/utils.py	Updates attribute checks to safely verify presence of qname.
pylint/checkers/deprecated.py	Adds a qname attribute check to avoid AttributeError.

Files not reviewed (1)

• doc/whatsnew/fragments/10334.bugfix: Language not supported

[DanielNoord]

Indeed, I would probably prefer to avoid all the hasattr calls. Is that possible?

[Pierre-Sassoulas]

The one I managed to avoid was guarded by a rather restrictive isinstance check, maybe we could do that instead. (It's protective but also we risk to have issue opened for it to add use case one by one if we were too protective). Or do you have another way to do it ?

[DanielNoord]

I think that would probably be the best course of action here?

[DanielNoord]

We can always change back to hasattr. Let's just try this first

[Pierre-Sassoulas]

My concern is that we might add a bunch of false negatives (even false positive ?) as we're affecting at least 20 messages or more (based on a quick look up of checker files where this helper is used). Not something we should risk in a patch (especially a x.y.6 patch which should be super stable imo). hasattr prevents the crash and has no side effect except being slower.

[DanielNoord]

Hmm I think the tests are probably a good indication that we are still covering the most common patterns. But up to you! I don't have a super strong preference other than that I do think we should use isinstance in the "final" version.

[cdce8p]

My concern is that we might add a bunch of false negatives (even false positive ?) as we're affecting at least 20 messages or more (based on a quick look up of checker files where this helper is used). Not something we should risk in a patch (especially a x.y.6 patch which should be super stable imo). hasattr prevents the crash and has no side effect except being slower.

Not sure that's really an issue here. The decorated_with functions checks if something is decorated with a specific decorator. If the inferred node isn't a ClassDef or FunctionDef, it's most likely not a decorator we're looking for. AFAICT from astroid, there are only a handful other classes with qname(). Mainly those for partial functions, lambdas and unknown. If we miss one of those, i.e. a new false-negative, so be it 🤷🏻‍♂️ If a true error arises, we can still go back and fix it with x.y.7. Should be fairly simple since we already now were to look.

[Pierre-Sassoulas]

Fair enough :) Added "A lot of checks that dealt with decorators (too many to list) are now shortcut if the decorator can't immediately be inferred to a function or class definition." to the changelog.

[github-actions]

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

This comment was generated for commit 6f7ab65

[github-actions]

The backport to maintenance/3.3.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-maintenance/3.3.x maintenance/3.3.x
# Navigate to the new working tree
cd .worktrees/backport-maintenance/3.3.x
# Create a new branch
git switch --create backport-10350-to-maintenance/3.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 77481a6e36fa7fa6b8530e9a300af840867e4df4
# Push it to GitHub
git push --set-upstream origin backport-10350-to-maintenance/3.3.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-maintenance/3.3.x

Then, create a pull request where the base branch is maintenance/3.3.x and the compare/head branch is backport-10350-to-maintenance/3.3.x.

[Pierre-Sassoulas]

Did the backport manually in #10361