Reduce intersections with conflicting privates, elaborate on reasons #37762
Conversation
# Conflicts: # tests/baselines/reference/mixinAccessModifiers.types
src/compiler/checker.ts
Outdated
| function elaborateNeverIntersection(errorInfo: DiagnosticMessageChain | undefined, type: IntersectionType) { | ||
| const neverProp = find(getPropertiesOfUnionOrIntersectionType(type), isDiscriminantWithNeverType); | ||
| if (neverProp) { | ||
| return chainDiagnosticMessages(errorInfo, Diagnostics.The_intersection_0_was_reduced_to_never_because_property_1_has_conflicting_types_in_some_constituents, | ||
| typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.NoTypeReduction), symbolToString(neverProp)); | ||
| } | ||
| const privateProp = find(getPropertiesOfUnionOrIntersectionType(type), isConflictingPrivateProperty); | ||
| if (privateProp) { | ||
| return chainDiagnosticMessages(errorInfo, Diagnostics.The_intersection_0_was_reduced_to_never_because_property_1_exists_in_multiple_constituents_and_is_private_in_some, | ||
| typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.NoTypeReduction), symbolToString(privateProp)); | ||
| } | ||
| return errorInfo; | ||
| } |
There was a problem hiding this comment.
Consider reporting more than the first conflict. Here's what I had.
function tryElaborateIfIntersectionReducedToNever(type: Type, currentErrorInfo: DiagnosticMessageChain | undefined) {
if (getObjectFlags(type) & ObjectFlags.IsNeverIntersection) {
const intersectionString = typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.PreserveVacuousIntersections);
const conflictingPropertyNames = map(filter(getPropertiesOfUnionOrIntersectionType(<IntersectionType>type), isDiscriminantWithNeverType), symbolName);
const maxNumProperties = 3;
const propertiesToReport = conflictingPropertyNames.length <= maxNumProperties
? conflictingPropertyNames
: [...conflictingPropertyNames.slice(0, maxNumProperties), "..."];
const propertiesString = propertiesToReport.join(", ");
const diag = conflictingPropertyNames.length === 1
? Diagnostics.The_type_never_was_reduced_from_the_intersection_0_Some_types_of_that_intersection_conflict_in_the_property_1_so_values_of_that_type_can_never_exist
: Diagnostics.The_type_never_was_reduced_from_the_intersection_0_Some_types_of_that_intersection_conflict_in_the_properties_1_so_values_of_that_type_can_never_exist;
return chainDiagnosticMessages(currentErrorInfo, diag, intersectionString, propertiesString);
}
return currentErrorInfo;
}There was a problem hiding this comment.
I looked at doing that but decided it wasn't worth the extra code and error messages. You feel strongly we need it?
There was a problem hiding this comment.
I was pretty back-and-forth on this. It's a few lines of code in the error case, and it can help users in the really complicated/borked situations. I don't feel that strongly about it, but it'd be a simple change.
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 8870838. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 8870838. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 8870838. You can monitor the build here. |
| @@ -15609,6 +15639,9 @@ namespace ts { | |||
| return result; | |||
| } | |||
| } | |||
| else if (target.flags & TypeFlags.Never && originalTarget.flags & TypeFlags.Intersection) { | |||
There was a problem hiding this comment.
You can actually just write
| else if (target.flags & TypeFlags.Never && originalTarget.flags & TypeFlags.Intersection) { | |
| else if (getObjectFlags(originalTarget) & ObjectFlags.IsNeverIntersection) { |
You can also move the check itself into a function that tries to elaborate on the current errorInfo and do an unconditional assignment (implementation was posted in one of my earlier comments.
| } | ||
|
|
||
| function f9(x: ProtectedGeneric<{a: void;}> & ProtectedGeneric<{a:void;b:void;}>) { | ||
| x.privateMethod(); // Error, private constituent makes method inaccessible | ||
| ~~~~~~~~~~~~~ | ||
| !!! error TS2546: Property 'privateMethod' has conflicting declarations and is inaccessible in type 'ProtectedGeneric<{ a: void; }> & ProtectedGeneric<{ a: void; b: void; }>'. | ||
| !!! error TS2341: Property 'privateMethod' is private and only accessible within class 'ProtectedGeneric<T>'. |
There was a problem hiding this comment.
This is an intended change. Error TS2546 is gone and has been superseded by the reduction of intersections with conflicting privates. In this particular case, however, the error wasn't even correct because there is only one declaration of privateMethod in the intersection (and therefore the intersection didn't get reduced to never).
| } | ||
| const privateProp = find(getPropertiesOfUnionOrIntersectionType(type), isConflictingPrivateProperty); | ||
| if (privateProp) { | ||
| return chainDiagnosticMessages(errorInfo, Diagnostics.The_intersection_0_was_reduced_to_never_because_property_1_exists_in_multiple_constituents_and_is_private_in_some, |
There was a problem hiding this comment.
Should we add related spans pointing at the location(s) of the private declarations? I imagine one of the possible "fixes" for this error is just making the fields public.
There was a problem hiding this comment.
We could consider it, but not super high value given that this is a pretty esoteric error.
|
@typescript-bot run dt |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 8870838. You can monitor the build here. |
In typescript 3.9.0, the intersection between two objects will be `never` if the two objects have incompatible private members. This broke an implicit behavior that `CombineObjects` was taking advantage of; element-wise intersection. Because `CombineObjects` is intended to be a thin wrapper over the raw intersection type, I don't want its contract to deviate from the intersection. So instead I added the `ElementwiseIntersect` utility; which relies on the newly added `TryKey` utility. `TryKey` is just like `GetKey`, except it fails "silently" when the key does not exist. Specifically, `GetKey` returns `never` so that the resultant type is unusable and `TryKey` returns `unknown` which can be eliminated via intersection. Relevant PR from typescript which changed behavior of intersections (and broke the future-proofing test cases): microsoft/TypeScript#37762
With this PR we reduce intersections with conflicting private property declarations to
never. Furthermore, we elaborate on the reasons forneverreduced intersections, explaining which property is causing the conflict.This PR supercedes #37724 and #37618 (stealing your thunder, @DanielRosenwasser).
Fixes #37659.