fix(solver): enforce nominal subtyping for NewType wrappers (#3081)

[Arths17]

Title

Fix NewType nominal handling for NoneType base (Fixes #3081)

Summary

This PR fixes a type-checking bug where a NewType defined over NoneType could be treated too much like its underlying type instead of preserving nominal behavior.

Specifically, NewType("NewNoneType", NoneType) should remain a distinct nominal type for static checking. Plain None must not satisfy a parameter annotated as NewNoneType unless explicitly wrapped.

Problem

Pyrefly was incorrectly normalizing parts of the NoneType NewType flow, which could lead to type confusion in analysis:

• NewType with a NoneType base was not consistently handled as a valid nominal wrapper in class metadata parsing.
• Validation logic could incorrectly reject this NewType shape via generic final/enum base checks.
• This made behavior around None versus NewNoneType inconsistent with nominal typing expectations.

What changed

1. Class metadata parsing update

• In AnswersSolver class metadata parsing, added a NewType-specific path for Type::None when is_new_type is true.
• This ensures NoneType-backed NewType definitions are interpreted through the intended nominal NewType pipeline.

2. Base validation adjustment

• Refined invalid-base checks so NewType with NoneType is not incorrectly rejected by final/enum inheritance guards.
• Keeps existing protections for other invalid NewType bases unchanged.

3. Regression coverage

• Added a dedicated regression test:
  • test_newtype_none_is_nominal
• Registered the new test module in the test suite index.

Behavior before

• NoneType-based NewType handling could be rejected or inconsistently treated during analysis.
• Nominal distinction for NewNoneType was not reliably preserved in the reported scenario.

Behavior after

• NewType("NewNoneType", NoneType) is accepted as a nominal NewType declaration.
• Plain None is rejected where NewNoneType is required.
• Wrapped values typed as NewNoneType are accepted.

Why this is correct

This aligns static behavior with PEP 484 nominal NewType semantics:

• NewType introduces a distinct nominal type for type checking.
• The underlying type informs runtime/value compatibility, but does not erase nominal identity at checking sites.

Test plan

• Added focused regression test for issue 3081 scenario.
• Ran targeted test:
  • cargo test --lib test::issue_3081_repro
• Ran full test suite:
  • cargo test

Both passed.

Risk and compatibility

• Low risk, scoped to NewType base parsing and NewType-specific base validation.
• No broad changes to union or general subtype machinery.
• Existing NewType tests continue to pass, plus new regression protection for NoneType-backed wrappers.

Suggested reviewer focus

• NewType base parsing branch for Type::None under is_new_type.
• The narrowed final/enum validation exception for the NoneType NewType case.
• Regression test intent and expected diagnostics.Continued with AutopilotProvided an enhanced PR description covering problem statement, root cause, exact code changes, before/after behavior, risk, and validated test plan for issue False negative: None incorrectly satisfies NewType('N', type[None]) #3081.

[Copilot]

Pull request overview

Fixes a solver edge case where NewType(..., NoneType) (and related normalized forms) could lose nominal behavior and/or be rejected during class metadata parsing, causing plain None to incorrectly satisfy a NewType wrapper.

Changes:

• Teach base-class parsing to treat Type::None as a valid NewType base by mapping it to the stdlib NoneType class.
• Refine invalid-base validation to avoid rejecting NewType over NoneType via final/enum inheritance guards.
• Add a regression test module for issue #3081 and register it in the test index.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pyrefly/lib/alt/class/class_metadata.rs	Adjusts NewType base parsing for Type::None and relaxes final/enum checks for NoneType NewType bases.
pyrefly/lib/test/issue_3081_repro.rs	Adds regression coverage ensuring None is not accepted where NewNoneType is required.
pyrefly/lib/test/mod.rs	Registers the new regression test module.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

is_none_type_newtype_base only checks for types.NoneType. In Python <3.10, Stdlib::none_type resolves to _typeshed.NoneType (also @final), and this branch can still be reached when the NewType base type gets canonicalized to Type::None (e.g. via type[None]). In that case, the final-class guard will still emit Cannot extend final class even though this should be permitted for NewType wrappers. Consider matching against self.stdlib.none_type().class_object() (or allowing both types.NoneType and _typeshed.NoneType) so the exception applies consistently across Python versions.

[Copilot]

The regression test covers NewType(..., NoneType), but the original issue repro uses NewType(..., type[None]). To ensure the reported scenario stays fixed (and to exercise the type[None] -> Type::None normalization path), consider adding an additional assertion/case in this test module that uses type[None] as the NewType base and verifies that test(None) is still rejected.

[rchen152]

Thanks for the PR! The general direction looks good. Left a few comments on test coverage and the final check being too narrow. It also looks like the tests failed to run due to a formatting error.

[rchen152]

Could you please put this test case in the existing new_type.rs test file?

[rchen152]

Please also add a test verifying that NewType("NewNoneType", type[None]) works as expected.

[Arths17]

Thanks for the detailed review! I've updated the PR to address all your feedback:

Test Consolidation: Moved the regression tests from the temporary module into the existing pyrefly/lib/test/new_type.rs file.

Expanded Coverage: Added an explicit test case for NewType("NewNoneType", type[None]) to ensure the normalization path is covered alongside NoneType.

Improved Compatibility: Updated the class_metadata check to use self.stdlib.none_type().class_object() instead of a narrow QName string. This should ensure it works correctly across different Python versions and _typeshed variations.

Lint/Format: Cleaned up the formatting to ensure all CI checks pass.

[rchen152]

Thanks! A couple more minor comments.

[rchen152]

You can combine this case and the previous one by doing Type::None | Type::Type(box Type::None) if is_new_type.

[rchen152]

Is this case necessary? It looks like type[None] is handled by the Type::Type(box Type::None) case you added below.

[Arths17]

Thanks for the follow-up nits! I've just pushed the fixes:

Consolidated Match Arms: Combined the Type::None and Type::Type(box Type::None) cases into a single arm using the | pattern.

Removed Redundancy: Removed the unnecessary duplicate case as confirmed.

The tests are still green and mypy_primer is showing that this actually improves compatibility for several open-source projects (like trio and spark) by correctly allowing NewType wrappers on final classes.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

colour (https://github.com/colour-science/colour)
- ERROR colour/hints/__init__.py:130:34-46: Cannot extend final class `RegexFlag` [invalid-inheritance]

core (https://github.com/home-assistant/core)
- ERROR homeassistant/components/matrix/__init__.py:79:50-60: Cannot extend final class `Pattern` [invalid-inheritance]

trio (https://github.com/python-trio/trio)
- ERROR src/trio/_core/_windows_cffi.py:23:28-33: Cannot extend final class `_CDataBase` [invalid-inheritance]
- ERROR src/trio/_core/_windows_cffi.py:24:38-43: Cannot extend final class `_CDataBase` [invalid-inheritance]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/sql/connect/_typing.py:56:86-98: Cannot extend final class `FunctionType` [invalid-inheritance]
- ERROR python/pyspark/sql/pandas/_typing/__init__.pyi:458:86-98: Cannot extend final class `FunctionType` [invalid-inheritance]

[github-actions]

Primer Diff Classification

✅ 4 improvement(s) | 4 project(s) total | -6 errors

4 improvement(s) across colour, core, trio, spark.

Project	Verdict	Changes	Error Kinds	Root Cause
colour	✅ Improvement	-1	invalid-inheritance	pyrefly/lib/alt/class/class_metadata.rs
core	✅ Improvement	-1	invalid-inheritance	is_final()
trio	✅ Improvement	-2	invalid-inheritance	is_final()
spark	✅ Improvement	-2	invalid-inheritance	is_final()

Detailed analysis

✅ Improvement (4)

colour (-1)

This is a clear improvement. The removed error was a false positive — NewType does not perform class inheritance, so the 'cannot extend final class' check should not apply. Per PEP 484 NewType, NewType creates a distinct nominal type, not a subclass. re.RegexFlag is an IntFlag enum, and some type checkers treat enum classes as implicitly final (restricting subclassing). However, NewType('RegexFlag', re.RegexFlag) is perfectly valid because it doesn't actually extend or subclass the class — it creates a callable that returns its argument at runtime and is treated as a distinct type by type checkers. The PR correctly gates the final/enum validation behind !is_new_type.
Attribution: The change in pyrefly/lib/alt/class/class_metadata.rs at lines 1348-1355 added the !is_new_type guard around the final/enum base validation check. Previously, when pyrefly processed NewType('RegexFlag', re.RegexFlag), it would see that re.RegexFlag (an enum) is final and emit invalid-inheritance. With the PR, the is_new_type flag causes this check to be skipped, correctly recognizing that NewType is not real class inheritance.

core (-1)

This is an improvement. The invalid-inheritance error was incorrectly applied to NewType declarations. Per the typing spec on NewType, NewType creates a distinct nominal type for static type checking — it does NOT create a subclass. Therefore, the @final restriction on re.Pattern should not prevent creating a NewType over it. The PR correctly skips the final/enum base validation when processing NewType declarations, removing this false positive.
Attribution: The change in pyrefly/lib/alt/class/class_metadata.rs at the base validation adjustment (lines around 1348-1355) is directly responsible. The condition was changed from if metadata.[is_final()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) || (metadata.[is_enum()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) && ...) to if !is_new_type && (metadata.[is_final()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) || (metadata.[is_enum()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) && ...)). This means when is_new_type is true, the final/enum base class check is skipped, which correctly removes the false positive invalid-inheritance error for NewType('ExpressionCommand', re.Pattern) since re.Pattern is final in typeshed.

trio (-2)

NewType is a typing construct that creates a nominal type wrapper — it does NOT create a subclass. Per PEP 484 (https://typing.readthedocs.io/en/latest/spec/glossary.html#term-NewType), NewType introduces a distinct type for static checking purposes. The invalid-inheritance / final-class check is meant for actual class inheritance (class Foo(FinalClass): ...), not for NewType('Foo', FinalClass). Pyrefly was incorrectly applying the final-class inheritance restriction to NewType declarations. CData resolves to _CDataBase in typeshed, which is marked @final, but since NewType doesn't actually extend the class, the check was a false positive. The PR correctly fixes this by adding !is_new_type guard before the final/enum base validation.
Attribution: The change in pyrefly/lib/alt/class/class_metadata.rs at lines 1351-1354 added the !is_new_type guard before the metadata.[is_final()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) check. Previously, when pyrefly processed NewType('Handle', CData), it would resolve CData to _CDataBase (which is final in typeshed), and then the final-class check would fire. With the new guard, NewType declarations skip this check entirely, which is correct because NewType doesn't create actual subclasses.

spark (-2)

These were false positives. The invalid-inheritance rule for final classes (per https://typing.readthedocs.io/en/latest/spec/class-compat.html#final-classes) applies to class inheritance (class Foo(FinalClass): ...), not to NewType declarations. NewType creates a nominal type wrapper, not a subclass. Pyrefly was incorrectly treating the NewType base type as a class base and applying the final-class check. The PR correctly skips this check when is_new_type is true.
Attribution: The change in pyrefly/lib/alt/class/class_metadata.rs at lines 1348-1355 (the if !is_new_type && (metadata.[is_final()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/class/class_metadata.rs) || ...) guard) is what fixed this. Previously, the final/enum base validation was applied unconditionally to all parsed base classes, including NewType bases. The PR added the !is_new_type condition so that NewType declarations skip the final-class and enum-base checks. This correctly removes the false positive invalid-inheritance errors for NewType('X', FunctionType) since FunctionType is @final.

---

Was this helpful? React with 👍 or 👎

_{Classification by primer-classifier (4 LLM)}

[rchen152]

Looks good, thanks!

[meta-codesync]

@rchen152 has imported this pull request. If you are a Meta employee, you can view this in D100284447.