-
Notifications
You must be signed in to change notification settings - Fork 13k
Consistently error on attempts to access private properties on generic objects #62300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Consistently error on attempts to access private properties on generic objects #62300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where TypeScript was inconsistently reporting errors when accessing private properties on generic type parameters. The fix ensures that attempts to access private properties on type parameters consistently produce error TS4105: "Private or protected member cannot be accessed on a type parameter."
- Moves the private property access check earlier in the indexed access type resolution process to catch more cases
- Extends test coverage to validate the fix across various generic type scenarios including unions, intersections, and mixed public/private property access patterns
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/cases/compiler/indexedAccessPrivateMemberOfGenericConstraint.ts | Adds comprehensive test cases for private property access on various generic type combinations |
tests/cases/compiler/indexedAccessPrivateMemberOfGenericConstraint2.ts | Creates new test file with additional scenarios involving object-typed private properties |
tests/baselines/reference/indexedAccessPrivateMemberOfGenericConstraint*.* | Updates baseline files to reflect the new consistent error reporting behavior |
src/compiler/checker.ts | Relocates and refactors the private property access validation logic to ensure comprehensive coverage |
if (!(type.flags & TypeFlags.IndexedAccess)) { | ||
return type; | ||
} | ||
// Check if the index type is assignable to 'keyof T' for the object type. | ||
const objectType = (type as IndexedAccessType).objectType; | ||
const indexType = (type as IndexedAccessType).indexType; | ||
|
||
if (!checkNonPublicPropertyAccess(objectType, indexType)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class A {
private declare a: { foo: number };
}
class B {
declare a: { bar: string };
}
type Test1<T extends A & B> = T["a"];
type Test2<T1 extends A, T2 extends B> = (T1 & T2)["a"];
The added branch related to never
is to account for the eager~ reduction of Test2
to never
. Because of that, checkIndexedAccessIndexType
wouldn't even have a chance to report the error.
In a similar fashion, the existing check had to be moved closer to the top to error at Test1
. At the current location, everyType(indexType, t => isTypeAssignableTo(t, objectIndexType)
would return before making this check. That's because objectIndexType
(keyof T
) would be mapped to its reduced apparent type when relating to it as the target. That would become keyof never
and that's just PropertyKey
- to which the "a"
source is assignable to.
fixes #62294
To be fair, I'm not sure if this is supposed to get fixed. I'm opening this as an experiment and with hope to understand better the team's opinion about
never
and accidental property access on it.First, #31942 introduced an error that was later removed in some of the cases by #37762 . This seamingly makes the errors more consistent but perhaps with intersections reduced to
never
all accesses should just be allowed. It's important to note here that to the user - in some of those cases - it's not apparent at all there is anynever
in sight. It may be sometimes spotted when hovering over a type alias containing it (if it's even a part of the type alias' "return type", and even if it is then it's still not always surfaced to the user there: TS playground) but one can't simply hover over an intersection to see that it reduces tonever
.At the very least, there is an existing small inconsistency in errors reported on union types (the reported error is order-dependent): TS playground. I patched that small here but I could also move it to a separate PR if this PR gets rejected.