reportPrivateUsage: honor __all__ for module-level _name in non-py.typed source#11375
Conversation
reportPrivateUsage previously honored __all__ for stub files and py.typed packages but not for plain source files. _bindNameValueToScope routed single-underscore module-scope names through _potentialPrivateSymbols (which consults __all__ at end-of-bind) only when isStubFile or isInPyTypedPackage was true, and called setIsPrivateMember() directly otherwise. This made the same symbol private or public depending on whether its file happened to be in include or reached via a python search path. Route through _potentialPrivateSymbols unconditionally so __all__ membership is honored consistently.
| @@ -47,6 +47,13 @@ test('Private1', () => { | |||
| TestUtils.validateResults(analysisResults, 4); | |||
| }); | |||
There was a problem hiding this comment.
Copilot generated:
-53
Missing test for _usesUnsupportedDunderAllForm interaction (High)
The test only covers the simple __all__ = ["..."] form. Add a test with a computed __all__ (e.g., __all__ = _base + ["public"]) and a _name import to validate the fallback behavior. This is the exact path where the regression identified above occurs.
There was a problem hiding this comment.
Added in bc747f4 — private5.py defines _name with __all__ = _base + ["public"] (computed form), private6.py imports it. Test expects 1 reportPrivateUsage error.
| // setIsPrivateMember() directly for non-stub, | ||
| // non-py.typed source files, which meant __all__ was | ||
| // honored only for stubs and py.typed packages. | ||
| this._potentialPrivateSymbols.set(name, symbol); |
There was a problem hiding this comment.
Copilot generated:
Regression when _usesUnsupportedDunderAllForm is true (High)
The old code at this location called setIsPrivateMember() directly for plain-source _name symbols. The new code defers them into _potentialPrivateSymbols, but the resolution loop at line 366 is guarded by !this._usesUnsupportedDunderAllForm. When a module uses a computed all form (e.g., all = _list + ["foo"]), the flag is true, the loop is skipped, and setIsPrivateMember() is never called — silently suppressing reportPrivateUsage for all _name imports from that module.
The Advocate acknowledged this as a genuine weakness. Consider falling back to setIsPrivateMember() for symbols in _potentialPrivateSymbols when _usesUnsupportedDunderAllForm is true, matching the old unconditional behavior. Add a test with a computed all form to cover this path.
There was a problem hiding this comment.
Thanks for flagging — I traced this and don't believe there's a regression here: the _potentialPrivateSymbols resolution loop (binder.ts ~L344) isn't gated on _usesUnsupportedDunderAllForm. For __all__ = _base + [...] the assignment handler sets _dunderAllNames = [], so !this._dunderAllNames?.some(...) is true and setIsPrivateMember() is called as before. This is also the pre-existing behavior for stub/py.typed packages, which already routed through this same path. Added private5.py/private6.py in bc747f4 covering exactly this case to lock it in — happy to add an explicit fallback if I'm misreading the loop.
| // Defer the private/protected decision until __all__ has | ||
| // been processed so an explicit __all__ entry can mark | ||
| // the symbol public. Previously this path called | ||
| // setIsPrivateMember() directly for non-stub, |
There was a problem hiding this comment.
info · L3590–3595
-3595
The comment is well-intentioned but includes 4 lines of historical narration ("Previously this path called setIsPrivateMember() directly..."). Per repo coding standards, prefer comments that explain why, not changelog-style what-was. The first two lines are good; trim to:
// Defer the private/protected decision until all is processed
// so an explicit all entry can promote the symbol to public.
this._potentialPrivateSymbols.set(name, symbol);
The historical context belongs in the commit message / PR description (where it already exists).
There was a problem hiding this comment.
Done in bc747f4 — trimmed to the two-line version.
| @@ -47,6 +47,13 @@ test('Private1', () => { | |||
| TestUtils.validateResults(analysisResults, 4); | |||
| }); | |||
There was a problem hiding this comment.
Copilot generated:
-53
Missing test for _usesUnsupportedDunderAllForm interaction (High)
The test only covers the simple __all__ = ["..."] form. Add a test with a computed __all__ (e.g., __all__ = _base + ["public"]) and a _name import to validate the fallback behavior. This is the exact path where the regression identified above occurs.
Adds private5.py/private6.py covering reportPrivateUsage when __all__ uses a computed form. The deferred resolution loop is not gated on _usesUnsupportedDunderAllForm; with a computed __all__ assignment _dunderAllNames is [], so the loop calls setIsPrivateMember() as before. This matches the existing stub/py.typed behavior the PR generalizes. Also trims the changelog-style comment per review.
|
Diff from mypy_primer, showing the effect of this PR on open source code: sympy (https://github.com/sympy/sympy)
- .../projects/sympy/sympy/stats/crv_types.py:2543:9 - error: Method "_cdf" overrides class "SingleContinuousDistribution" in an incompatible manner
- Return type mismatch: base method returns type "None", override returns type "ComplexInfinity | NaN | Rational | Unknown"
- Type "ComplexInfinity | NaN | Rational | Unknown" is not assignable to type "None"
- "ComplexInfinity" is not assignable to "None" (reportIncompatibleMethodOverride)
- .../projects/sympy/sympy/stats/crv_types.py:2722:9 - error: Method "_cdf" overrides class "SingleContinuousDistribution" in an incompatible manner
- Return type mismatch: base method returns type "None", override returns type "Expr | NaN | ComplexInfinity | Rational | Unknown"
- Type "Expr | NaN | ComplexInfinity | Rational | Unknown" is not assignable to type "None"
- "Expr" is not assignable to "None" (reportIncompatibleMethodOverride)
- .../projects/sympy/sympy/stats/frv_types.py:139:17 - error: Argument of type "NaN | ComplexInfinity | Rational | Unknown | Expr" cannot be assigned to parameter "value" of type "int" in function "__setitem__"
- Type "NaN | ComplexInfinity | Rational | Unknown | Expr" is not assignable to type "int"
- "Expr" is not assignable to "int" (reportArgumentType)
- .../projects/sympy/sympy/stats/symbolic_probability.py:75:9 - error: Method "doit" overrides class "Basic" in an incompatible manner
- Return type mismatch: base method returns type "Basic", override returns type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]"
- Type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" is not assignable to type "Basic"
- "Literal[0]" is not assignable to "Basic" (reportIncompatibleMethodOverride)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1543:12 - error: No overloads for "simplify" match the provided arguments (reportCallIssue)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1543:21 - error: Argument of type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
- Type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" is not assignable to type "Basic"
- "Literal[0]" is not assignable to "Basic" (reportArgumentType)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1545:12 - error: No overloads for "simplify" match the provided arguments (reportCallIssue)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1545:21 - error: Argument of type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
- Type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" is not assignable to type "Basic"
- "Literal[0]" is not assignable to "Basic" (reportArgumentType)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1550:12 - error: No overloads for "simplify" match the provided arguments (reportCallIssue)
- .../projects/sympy/sympy/stats/tests/test_continuous_rv.py:1550:21 - error: Argument of type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" cannot be assigned to parameter "expr" of type "Basic" in function "simplify"
- Type "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" is not assignable to type "Basic"
- "Literal[0]" is not assignable to "Basic" (reportArgumentType)
- .../projects/sympy/sympy/stats/tests/test_finite_rv.py:34:23 - error: Operator "/" not supported for types "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]"
+ .../projects/sympy/sympy/stats/tests/test_finite_rv.py:34:23 - error: Operator "/" not supported for types "Unknown | Any | BernoulliDistribution | Probability | Zero | One | Expr | Float | ComplexInfinity | NaN | Rational | Infinity | NegativeInfinity | NotImplementedType | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | Integer | Lambda | Mul | NegativeOne | Number | Integral | Literal[0]" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | Expr | Float | ComplexInfinity | NaN | Rational | Infinity | NegativeInfinity | NotImplementedType | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | Integer | Lambda | Mul | NegativeOne | Number | Integral | Literal[0]"
- .../projects/sympy/sympy/stats/tests/test_finite_rv.py:35:23 - error: Operator "*" not supported for types "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]"
+ .../projects/sympy/sympy/stats/tests/test_finite_rv.py:35:23 - error: Operator "*" not supported for types "Unknown | Any | BernoulliDistribution | Probability | Zero | One | Expr | Float | ComplexInfinity | NaN | Rational | Infinity | NegativeInfinity | NotImplementedType | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | Integer | Lambda | Mul | NegativeOne | Number | Integral | Literal[0]" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | Expr | Float | ComplexInfinity | NaN | Rational | Infinity | NegativeInfinity | NotImplementedType | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | Integer | Lambda | Mul | NegativeOne | Number | Integral | Literal[0]"
- Operator "*" not supported for types "BernoulliDistribution" and "tuple[Unknown, ...]"
- Operator "*" not supported for types "BernoulliDistribution" and "Sum"
- Operator "*" not supported for types "BernoulliDistribution" and "ZeroMatrix"
+ Operator "*" not supported for types "BernoulliDistribution" and "Float"
+ Operator "*" not supported for types "BernoulliDistribution" and "ComplexInfinity"
+ Operator "*" not supported for types "BernoulliDistribution" and "Rational"
- Operator "*" not supported for types "BernoulliDistribution" and "Piecewise"
+ Operator "*" not supported for types "BernoulliDistribution" and "Infinity"
- .../projects/sympy/sympy/stats/tests/test_finite_rv.py:35:23 - error: Operator "/" not supported for types "Unknown | Any | One | NegativeOne | Zero | Integer | NaN | ComplexInfinity | Rational | Expr | MatMul | tuple[Unknown, ...] | Infinity | NegativeInfinity | int" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | tuple[Unknown, ...] | Sum | Expr | ZeroMatrix | NaN | Piecewise | Basic | ComplexInfinity | Float | Infinity | Integer | Lambda | Mul | NegativeInfinity | NegativeOne | Number | Rational | Integral | Literal[0]"
+ .../projects/sympy/sympy/stats/tests/test_finite_rv.py:35:23 - error: Operator "/" not supported for types "Unknown | Any | One | NegativeOne | Zero | Integer | NaN | ComplexInfinity | Rational | Expr | MatMul | Infinity | NegativeInfinity | Float | NotImplementedType | tuple[Unknown, ...] | int" and "Unknown | Any | BernoulliDistribution | Probability | Zero | One | Expr | Float | ComplexInfinity | NaN | Rational | Infinity | NegativeInfinity | NotImplementedType | tuple[Unknown, ...] | Sum | ZeroMatrix | Piecewise | Basic | Integer | Lambda | Mul | NegativeOne | Number | Integral | Literal[0]"
... (truncated 583 lines) ...
|
reportPrivateUsageconsults__all__for module-level single-underscore names only when the declaring file is bound as a stub or a py.typed package. For plain source files,_bindNameValueToScopecallssetIsPrivateMember()directly, so an explicit__all__entry does not suppress the diagnostic.This means the same symbol is reported or not depending on whether its file is in
include:This PR routes the non-stub, non-py.typed path through
_potentialPrivateSymbolsas well, so__all__is honored consistently.The new
private3.py/private4.pysample test fails before the change (2 errors) and passes after (1 error - only the name not in__all__).