Skip to content

fix Do not enforce name alignment for namedtuple and enum functional syntax #2874#2892

Open
asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
asukaminato0721:2874
Open

fix Do not enforce name alignment for namedtuple and enum functional syntax #2874#2892
asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
asukaminato0721:2874

Conversation

@asukaminato0721
Copy link
Contributor

Summary

Fixes #2874

no longer enforces assignment-name alignment for functional Enum and functional NamedTuple definitions.

Test Plan

update && add test

Copilot AI review requested due to automatic review settings March 25, 2026 03:37
@meta-cla meta-cla bot added the cla signed label Mar 25, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes issue #2874 by removing the diagnostic that enforced assignment-name alignment for functional Enum(...) and NamedTuple(...)/namedtuple(...) definitions, matching the behavior of other modern type checkers.

Changes:

  • Stop emitting “Expected string literal <varname>” errors for functional Enum definitions whose first string argument doesn’t match the assignment target name.
  • Stop emitting the same alignment error for functional typing.NamedTuple(...) and collections.namedtuple(...) assignments (while still binding/type-checking the passed name expression).
  • Add regression tests covering name-mismatch cases for both functional namedtuple and functional Enum.

Reviewed changes

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

File Description
pyrefly/lib/binding/stmt.rs Removes name-alignment enforcement for functional namedtuple assignments; still type-checks the name expression via ensure_expr.
pyrefly/lib/binding/class.rs Removes name-alignment enforcement for functional Enum synthesis.
pyrefly/lib/test/named_tuple.rs Adds regression test ensuring mismatched functional namedtuple names don’t error and types are still inferred as expected.
pyrefly/lib/test/enums.rs Adds regression test ensuring mismatched functional Enum names don’t error and member literals are still inferred.

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link

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

dd-trace-py (https://github.com/DataDog/dd-trace-py)
- ERROR ddtrace/vendor/psutil/_psbsd.py:868:9-15: Expected string literal "nt_mmap_grouped" [invalid-argument]
- ERROR ddtrace/vendor/psutil/_psbsd.py:871:9-15: Expected string literal "nt_mmap_ext" [invalid-argument]
- ERROR ddtrace/vendor/psutil/_pssunos.py:670:34-40: Expected string literal "nt_mmap_grouped" [invalid-argument]
- ERROR ddtrace/vendor/psutil/_pssunos.py:671:30-36: Expected string literal "nt_mmap_ext" [invalid-argument]

pycryptodome (https://github.com/Legrandin/pycryptodome)
- ERROR lib/Crypto/Cipher/_mode_gcm.py:70:28-40: Expected string literal "GHASH_Imp" [invalid-argument]

scikit-learn (https://github.com/scikit-learn/scikit-learn)
- ERROR sklearn/utils/_testing.py:840:23-29: Expected string literal "Args" [invalid-argument]

psycopg (https://github.com/psycopg/psycopg)
- ERROR tests/types/test_enum.py:351:9-18: Expected string literal "enum" [invalid-argument]

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
- ERROR pymongo/ocsp_cache.py:50:9-31: Expected string literal "CACHE_KEY_TYPE" [invalid-argument]

cloud-init (https://github.com/canonical/cloud-init)
- ERROR tests/unittests/sources/test_smartos.py:793:9-17: Expected string literal "res" [invalid-argument]
- ERROR tests/unittests/test_net_activators.py:48:24-31: Expected string literal "mocks" [invalid-argument]
- ERROR tests/unittests/test_net_activators.py:67:24-31: Expected string literal "mocks" [invalid-argument]

pandas (https://github.com/pandas-dev/pandas)
- ERROR pandas/core/frame.py:1440:17-21: Expected string literal "itertuple" [invalid-argument]
- ERROR pandas/tests/frame/indexing/test_indexing.py:1407:36-45: Expected string literal "indexer_tuple" [invalid-argument]
- ERROR pandas/tests/frame/test_constructors.py:1613:34-42: Expected string literal "named_tuple" [invalid-argument]

discord.py (https://github.com/Rapptz/discord.py)
- ERROR discord/enums.py:96:22-42: Expected string literal "cls" [invalid-argument]

spack (https://github.com/spack/spack)
- ERROR lib/spack/spack/test/test_suite.py:265:40-51: Expected string literal "MockSuite" [invalid-argument]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/serializers.py:356:42-46: Expected string literal "cls" [invalid-argument]

@github-actions
Copy link

Primer Diff Classification

✅ 9 improvement(s) | ➖ 1 neutral | 10 project(s) total | -17 errors

9 improvement(s) across dd-trace-py, pycryptodome, scikit-learn, psycopg, mongo-python-driver, pandas, discord.py, spack, spark.

Project Verdict Changes Error Kinds Root Cause
dd-trace-py ✅ Improvement -4 invalid-argument check_functional_definition_name()
pycryptodome ✅ Improvement -1 invalid-argument pyrefly/lib/binding/class.rs
scikit-learn ✅ Improvement -1 invalid-argument check_functional_definition_name()
psycopg ✅ Improvement -1 invalid-argument check_functional_definition_name()
mongo-python-driver ✅ Improvement -1 invalid-argument pyrefly/lib/binding/class.rs
cloud-init ➖ Neutral -3 invalid-argument check_functional_definition_name()
pandas ✅ Improvement -3 invalid-argument check_functional_definition_name()
discord.py ✅ Improvement -1 invalid-argument pyrefly/lib/binding/class.rs
spack ✅ Improvement -1 invalid-argument check_functional_definition_name()
spark ✅ Improvement -1 invalid-argument check_functional_definition_name()
Detailed analysis

✅ Improvement (9)

dd-trace-py (-4)

The removed errors were false positives — they flagged valid Python code as having an 'invalid argument'. Specifically, nt_mmap_grouped = namedtuple('mmap', ...) and nt_mmap_ext = namedtuple('mmap', ...) use 'mmap' as the namedtuple typename, which doesn't match the variable names 'nt_mmap_grouped' and 'nt_mmap_ext'. The error messages "Expected string literal 'nt_mmap_grouped'" and "Expected string literal 'nt_mmap_ext'" indicate pyrefly was enforcing that the first argument to namedtuple() must match the variable name it's assigned to. However, this is not a type error — Python's namedtuple() accepts any valid string as its first argument (the typename), and there is no requirement that it match the variable name. The code is perfectly valid and will execute correctly. The mismatch between variable name and typename is at most a style/convention issue, not a type error. The PR correctly removes these false positive invalid-argument errors.
Attribution: The change in pyrefly/lib/binding/class.rs in check_functional_definition_name() now accepts an error_kind parameter. For namedtuple functional definitions in pyrefly/lib/binding/stmt.rs, the error kind was changed from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName. Since UnexpectedName has Severity::Warn (set in crates/pyrefly_config/src/error_kind.rs), these name-mismatch diagnostics are now warnings instead of errors, which is why the invalid-argument errors disappeared.

pycryptodome (-1)

The removed invalid-argument error on GHASH_Imp = namedtuple('_GHash_Imp', funcs) was a false positive. The error message Expected string literal "GHASH_Imp" indicates the type checker was enforcing that the first argument to namedtuple() must match the variable name it's assigned to. While this is a common convention (and recommended by PEP 8 and mypy), it is not a requirement of Python — the code is valid and works correctly at runtime. The variable GHASH_Imp holds a namedtuple class whose internal __name__ is _GHash_Imp, which is a deliberate choice by the pycryptodome authors (using a leading underscore to indicate the class is an implementation detail). Removing this invalid-argument error is an improvement because the mismatch between the variable name and the namedtuple's typename argument is not an invalid argument — the first argument to namedtuple is a valid string, and the call succeeds. Whether the PR introduces a softer warning (like unexpected-name) or simply suppresses the diagnostic entirely is not determinable from the provided information, but removing the error-level diagnostic is correct.
Attribution: The change in pyrefly/lib/binding/class.rs in the check_functional_definition_name method now accepts an error_kind parameter. For namedtuple definitions (in pyrefly/lib/binding/stmt.rs), the error kind was changed from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName. The UnexpectedName error kind has Severity::Warn (defined in pyrefly/crates/pyrefly_config/src/error_kind.rs), which is lower than the previous InvalidArgument severity. This reclassification caused the invalid-argument error to disappear from the primer results for this file.

scikit-learn (-1)

The original error Expected string literal "Args" [invalid-argument] at line 840 flags that in Args = namedtuple("args", ["include", "exclude", "arg_name"]), the first argument to namedtuple() is "args" (lowercase) but the variable it's assigned to is Args (capitalized). The type checker expected the string literal to match the variable name, i.e., "Args". This is a genuine name mismatch in a functional namedtuple definition. The - line in the diff indicates this invalid-argument error is being removed. The code itself is valid Python — namedtuple does not require the name string to match the variable name, though it's conventional for them to match. The PR removes this error, likely because pyrefly changed how it handles namedtuple functional definitions where the string argument doesn't match the assigned variable name. This is a reasonable change since the mismatch doesn't cause runtime errors or type unsafety — the namedtuple will simply have __name__ as "args" while being bound to the variable Args.
Attribution: The change in pyrefly/lib/binding/stmt.rs at lines 606-629 changed the error kind passed to check_functional_definition_name() from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName for both typing.NamedTuple and collections.namedtuple functional definitions. The corresponding change in pyrefly/lib/binding/class.rs at line 1001 also changed the enum functional definition to use UnexpectedName. The new UnexpectedName error kind was added in crates/pyrefly_config/src/error_kind.rs with Severity::Warn default. This reclassification caused the old [invalid-argument] error to disappear (it's now reported as [unexpected-name] instead, but that new error isn't shown in the diff, likely because it's a warning rather than an error).

psycopg (-1)

The PR reclassifies the name-mismatch check for functional Enum and NamedTuple definitions from invalid-argument (error severity) to unexpected-name (warning severity). The removed error was flagging Enum("ByValue", ...) assigned to variable enum — the type checker expected the first string argument to match the variable name enum, but found "ByValue" instead. This is a legitimate name mismatch, but one that's common in practice and doesn't indicate a type error. The code already had # type: ignore on that line, acknowledging the intentional mismatch. Reclassifying this as a warning rather than an error is a reasonable improvement in error categorization — it reduces noise for a check that is stylistic rather than type-safety-related.
Attribution: The change in pyrefly/lib/binding/class.rs in the check_functional_definition_name() method and its callers changed the error kind from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName for functional Enum definitions (in bind_functional_enum_def). The new UnexpectedName error kind defined in pyrefly/crates/pyrefly_config/src/error_kind.rs has Severity::Warn as its default. This means the name mismatch check for functional Enum definitions is now a warning instead of an error, which is why the invalid-argument error disappears from the primer output — it's been reclassified to a lower-severity unexpected-name warning that may not appear in the primer results.

mongo-python-driver (-1)

The removed invalid-argument error on CACHE_KEY_TYPE = namedtuple('OcspResponseCacheKey', ...) was flagging a mismatch between the variable name CACHE_KEY_TYPE and the string argument 'OcspResponseCacheKey'. While type checkers like mypy also flag this pattern (since the string argument determines the actual __name__ of the resulting class, and a mismatch can cause confusion with pickling and repr), using a different string name in a functional namedtuple definition is a common, intentional pattern in real-world code — the variable name serves as the class attribute name while the string determines the type's __name__. The PR removes this invalid-argument error. This is an improvement because the error-level diagnostic was too strict for this valid and widely-used code pattern, which the original code even acknowledges with a # type: ignore comment.
Attribution: The change in pyrefly/lib/binding/class.rs in the check_functional_definition_name method now accepts an error_kind parameter. For collections.namedtuple definitions (in pyrefly/lib/binding/stmt.rs around line 606-629), the error kind was changed from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName. Since UnexpectedName has Severity::Warn (set in crates/pyrefly_config/src/error_kind.rs), the pymongo error at line 50 is downgraded from an error to a warning, which means it no longer appears in the error output.

pandas (-3)

The PR correctly reclassifies namedtuple/enum name-mismatch checks from InvalidArgument (error) to UnexpectedName (warning). The removed errors were false positives. At frame.py:1440, the name parameter (typed str | None) is passed as the first argument to collections.namedtuple(). The type checker flagged this because it expects a string literal, but name is a variable — this is a known limitation also noted by the existing mypy suppression comment on line 1437-1438. The code is perfectly valid Python at runtime. The other two test cases similarly pass string variables where the checker expects string literals. These are at most style warnings about the namedtuple/enum typename not matching the variable name, not actual type errors, making the reclassification from error to warning appropriate.
Attribution: The change to check_functional_definition_name() in pyrefly/lib/binding/class.rs and pyrefly/lib/binding/stmt.rs changed the error kind for namedtuple/enum functional definitions from ErrorKind::InvalidArgument (Error severity) to ErrorKind::UnexpectedName (Warn severity, as set in crates/pyrefly_config/src/error_kind.rs). This severity downgrade caused these errors to no longer appear in the primer output.

discord.py (-1)

This is a clear improvement. The removed error was a false positive — cls = namedtuple('_EnumValue_' + name, 'name value') creates a namedtuple where the type name is dynamically computed and intentionally differs from the variable name cls. The type checker was enforcing that the first argument to namedtuple() must match the variable name it's assigned to (expecting 'cls'), but this is an overly strict check for this use case. The code is a helper function that creates namedtuple classes with names like _EnumValue_ChannelType at runtime — the variable cls is just a temporary local that gets returned. The PR correctly removes this false positive by relaxing the name-matching requirement for namedtuple functional syntax when the name is a dynamic expression rather than a simple string literal.
Attribution: The change in pyrefly/lib/binding/class.rs in the check_functional_definition_name method changed the error kind from ErrorKind::InvalidArgument to a parameterized error_kind. For namedtuple functional definitions (called from pyrefly/lib/binding/stmt.rs), the error kind is now ErrorKind::UnexpectedName, which has Severity::Warn (set in crates/pyrefly_config/src/error_kind.rs). Previously it was ErrorKind::InvalidArgument which had error severity. The namedtuple('_EnumValue_' + name, 'name value') call on line 96 of discord/enums.py has a non-string-literal first argument, which triggers the else branch in check_functional_definition_name. This branch now emits UnexpectedName (warning) instead of InvalidArgument (error), so the ERROR-level diagnostic disappears.

spack (-1)

This is an improvement. The removed error was flagging MockSuite = collections.namedtuple("TestSuite", ["specs"]) as an invalid-argument error because the variable name MockSuite doesn't match the namedtuple typename argument "TestSuite". Pyrefly was expecting the first argument to be "MockSuite" to match the variable name. However, using a different variable name than the namedtuple's internal typename is a legitimate and common Python pattern, especially in test code where mock objects are created with intentionally different names. Neither mypy nor pyright flag this pattern. Removing this false positive error makes pyrefly less noisy and more aligned with other type checkers.
Attribution: The PR changed check_functional_definition_name() in pyrefly/lib/binding/class.rs and pyrefly/lib/binding/stmt.rs to use ErrorKind::UnexpectedName (severity: Warn) instead of ErrorKind::InvalidArgument for namedtuple and enum functional definitions. The new UnexpectedName error kind is defined in crates/pyrefly_config/src/error_kind.rs with Severity::Warn. This means the name mismatch check for collections.namedtuple() on line 265 of test_suite.py is now a warning rather than an error, removing it from the error output.

spark (-1)

The removed error was a false positive. The code cls = collections.namedtuple(name, fields) at line 356 is inside a helper function _restore() where name is a runtime parameter variable, not a string literal. This is dynamic namedtuple construction, not the functional class definition pattern like Point = namedtuple('Point', ['x', 'y']). The original error Expected string literal "cls" indicates pyrefly was checking whether the first argument to namedtuple() matches the assignment target name cls, but this check is only meaningful when the first argument is a string literal in a functional-form class definition. Here, name is a variable containing a dynamic value, so the check should not apply. The PR correctly removes this false positive - pyrefly now handles the case where the first argument is not a string literal, avoiding the spurious error on legitimate dynamic namedtuple usage.
Attribution: The changes in pyrefly/lib/binding/stmt.rs modified how functional NamedTuple definitions are processed. The error kind was changed from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName for both collections.namedtuple and typing.NamedTuple functional definitions. Since UnexpectedName has Severity::Warn (which may be configured to be suppressed), this could explain the removal. Additionally, the addition of self.ensure_expr(arg_name, &mut Usage::StaticTypeInformation) may have changed how the argument expression is evaluated. The changes in pyrefly/lib/binding/class.rs to check_functional_definition_name() now accept an error_kind parameter, allowing different error kinds for different contexts.

➖ Neutral (1)

cloud-init (-3)

This is a neutral change — the PR reclassifies the error kind from invalid-argument to unexpected-name for functional namedtuple and Enum definitions. The underlying check (name mismatch between variable and first string argument) is preserved; only the error category changed. The removed invalid-argument errors should be replaced by unexpected-name warnings. This is a message/classification change with no behavioral impact on what gets flagged.
Attribution: The changes in pyrefly/lib/binding/class.rs (in check_functional_definition_name()) and pyrefly/lib/binding/stmt.rs changed the error kind parameter from ErrorKind::InvalidArgument to ErrorKind::UnexpectedName for namedtuple (both collections.namedtuple and typing.NamedTuple) and Enum functional definitions. The new ErrorKind::UnexpectedName is defined in crates/pyrefly_config/src/error_kind.rs with Severity::Warn. This reclassification means the old invalid-argument errors disappear and are replaced by unexpected-name warnings. The primer output only shows the removed invalid-argument errors; the new unexpected-name warnings likely aren't shown because they're at warning severity or because the primer diff only captures the removal side.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (10 LLM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not enforce name alignment for namedtuple and enum functional syntax

2 participants