Skip to content

Conversation

emmanuel-ferdman
Copy link
Contributor

Type of Changes

Type
🔨 Refactoring

Description

This PR improves error reporting for type.__new__() validation by replacing silent None returns with descriptive InferenceError exceptions. Previously, when type.__new__() received invalid arguments (wrong metaclass type, invalid name, bad bases, etc.), the code would return None without explanation. Now it raises InferenceError with clear messages like "type.new() requires a class for metaclass" or "type.new() requires 4 arguments, got 3". This makes debugging easier and brings the validation in line with other astroid error handling patterns. All existing tests pass without modification since they already expected InferenceError for these cases.

This is part of the on-going work in PR #2847.

Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.37%. Comparing base (019e2b2) to head (1688ef9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2851   +/-   ##
=======================================
  Coverage   93.37%   93.37%           
=======================================
  Files          92       92           
  Lines       11148    11150    +2     
=======================================
+ Hits        10409    10411    +2     
  Misses        739      739           
Flag Coverage Δ
linux 93.23% <100.00%> (+<0.01%) ⬆️
pypy 93.37% <100.00%> (+<0.01%) ⬆️
windows 93.35% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
astroid/bases.py 89.69% <100.00%> (+0.05%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we can test this?

I do think this deserves a Changelog entry

@emmanuel-ferdman
Copy link
Contributor Author

@DanielNoord The existing tests (test_type__new__invalid_mcs_argument, test_type__new__invalid_name, test_type__new__invalid_bases) cover these changes. While we added descriptive error messages in the code for developer clarity, they (at least currently) don't propagate to end users because Call._infer() catches all InferenceError exceptions. Even pylint currently siliently ignores the InferenceError error. The important improvement is that we now raise errors immediately instead of returning None and falling through to potentially incorrect inference paths.

I have added the changelog, hopefully in the correct place 😄

@DanielNoord DanielNoord merged commit 2ce47a2 into pylint-dev:main Oct 7, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants