Shared interface members accidentally exposed through analyzer public API #59763

stereotype441 · 2024-12-19T20:11:26Z

The following members were accidentally added to the analyzer's public API, by virtue of the fact that some types now mention a shared interface class in their implements clause:

FunctionType.positionalParameterTypes (exposed via SharedFunctionTypeStructure)
FunctionType.requiredPositionalParameterCount (exposed via SharedFunctionTypeStructure)
FunctionType.sortedNamedParameters (exposed via SharedFunctionTypeStructure)
RecordType.positionalTypes (exposed via SharedRecordTypeStructure)
RecordType.sortedNamedTypes (exposed via SharedRecordTypeStructure)

After discussing this with @bwilkerson, I've decided to try to address this by:

Moving the reference to SharedFunctionTypeStructure from the implements clause of FunctionType to the implements clause of FunctionTypeImpl (so that the members of SharedFunctionTypeStructure won't be exposed to analyzer clients using FunctionType)
Moving the reference to SharedRecordTypeStructure from the implements clause of RecordType to the implements clause of RecordTypeImpl (so that the members of SharedFunctionTypeStructure won't be exposed to analyzer clients using RecordType)

To avoid breaking clients at the time that I make these changes, I'll add deprecated declarations of these members to the FunctionType and RecordType classes. This will ensure that if any analyzer clients have already started depending on these accidentally-exposed members, their code will continue to work, but they'll be alerted to the fact that the members will be removed in the future.

To prevent this sort of problem from cropping up in the future, I intend to change all the analyzer's uses of the shared interface classes in a similar way (e.g., TypeImpl will implement SharedTypeStructure instead of DartType). To make this possible, I'll first need to change the analyzer's use of _fe_analyzer_shared so that when instantiating shared classes, it always does supplies non-public types for the type arguments. (E.g., ResolverVisitor will mix in TypeAnalyzer<TypeImpl, AstNodeImpl, StatementImpl, ...> instead of TypeAnalyzer<DartType, AstNode, Statement, ...>).

I'll post updates here if my plans change.

The text was updated successfully, but these errors were encountered:

… code. Change the analyzer's use of shared classes and mixins to to supply `PromotableElementImpl2` instead of `PromotableElement2` as the type parameter that represents promotable variables. This is a follow-up to https://dart-review.googlesource.com/c/sdk/+/400660, which changed the code from supplying `PromotableElement` to `PromotableElement2`. That change was both larger and riskier than this one, since it changed the identity of the objects that were passed to shared analysis code. This change merely substitutes a more precise type for the type argument, without changing the underlying objects in use. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I2013dd0b7d38350ed7a9f2f4bd875d0006941754 Bug: #59763 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401740 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

…public API. When I introduced the `SharedFunctionTypeStructure` class in https://dart-review.googlesource.com/c/sdk/+/386322, I failed to realize that a side effect of this change was to expose the shared methods `positionalParameterTypes`, `requiredPositionalParameterCount`, and `sortedNamedParameters` through the analyzer's public API. To correct that mistake, I've moved the reference to `SharedFunctionTypeStructure` from the `implements` clause of `FunctionType` to the `implements` clause of `FunctionTypeImpl`. To avoid this change causing a breakage for clients, I've also added deprecated declarations of these three getters to the `FunctionType` class. This ensures that if any analyzer clients have already started depending on them, their code will continue to work, but they'll be alerted to the fact that the members will be removed in the future. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Bug: #59763 Change-Id: I442cbe29ed938ec2fed3a3fa65a95c9f0f47a38d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401921 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The following adjustements are made to the analyzer class hierarchy: - `SharedNamedFunctionParameterStructure` is moved from the `implements` clause of `ParameterElement` to the implements clause of `ParameterElementMixin`, a mixin that is used by all concrete subtypes of `ParameterElement`, but which is not part of the analyzer's public API. - `SharedNamedTypeStructure` is moved from the `implements` clause of `RecordTypeNamedField` to the implements clause of `RecordTypeNamedFieldImpl`, the sole concrete subtype of `RecordTypeNamedField`, which is not part of the analyzer's public API. To account for these changes, the analyzer's use of the shared types `FunctionTypeImpl`, `TypeConstraintGenerator`, and `TypeConstraintGeneratorMixin` needed to be adjusted so `ParameterElementMixin` is supplied as a type parameter instead of `ParameterElement`. This required adding a cast to the `FunctionTypeImpl` constructor, which I intend to remove in a future CL. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I889b9f87d7d077d10935c4506c454507a480afcd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402380 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

… code. Change the analyzer's use of the following shared generic types so that it supplies the type parameter `TypeParameterElementImpl2` instead of `TypeParameterElement` as the type it uses to represent type parameters: - `GeneratedTypeConstraint` - `MergedTypeConstraint` - `SharedFunctionTypeStructure` - `SharedInferenceLogWriter` - `SharedInferenceLogWriterImpl` - `TypeAnalyzer` - `TypeAnalyzerOperations` - `TypeConstraintFromArgument` - `TypeConstraintFromExtendsClause` - `TypeConstraintFromFunctionContext` - `TypeConstraintFromReturnType` - `TypeConstraintGenerator` - `TypeConstraintGeneratorMixin` - `TypeConstraintOrigin` - `UnknownTypeConstraintOrigin` This required adding type casts in a few places. In other places, I avoided type casts by tightening up some return types (for example, `InstanceElementImpl2.typeParameters2` now returns `List<TypeParameterElementImpl2>` instead of `List<TypeParameterElement2>`. The analyzer public API was unchanged, though (for example, `InstanceElement2.typeParameters2` still has a return type of `List<TypeParameterElement2>`), so clients should be unaffected. As a result of this change, it is no longer necessary for a public API class to implement the shared interface `SharedTypeParameterStructure`. This interface was previously implemented by `TypeParameterElement`; it is now implemented by `TypeParameterElementImpl2`, which is not a part of the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Ic6d42b415620430210e25ed212b11e03851d25ed Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402500 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

Change the analyzer's use of the following shared generic types so that it supplies the type parameter `InterfaceElementImpl2` instead of `InterfaceElement` as the type it uses to represent interfaces: - `MergedTypeConstraint` - `TypeAnalyzer` - `TypeAnalyzerOperations` - `TypeAnalyzerOperationsMixin` - `TypeConstraintFromArgument` - `TypeConstraintFromExtendsClause` - `TypeConstraintFromFunctionContext` - `TypeConstraintFromReturnType` - `TypeConstraintGenerator` - `TypeConstraintGeneratorMixin` - `TypeConstraintOrigin` - `TypeDeclarationMatchResult` - `UnknownTypeConstraintOrigin` To avoid type casts, I tightened up the return type of `InterfaceTypeImpl.element3` so that it returns `InterfaceElementImpl2` instead of `InterfaceElement2`. The analyzer public API is unchanged, though--`InterfaceType.element3` still has a return type of `InterfaceElement2`--so clients should be unaffected. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Ie13fcb31d8dfe0a2a256eda177a8c4a1a022008d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402428 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

…ucture. When I introduced the `SharedRecordType` class in https://dart-review.googlesource.com/c/sdk/+/362481 (now called `SharedRecordTypeStructure`), I failed to realize that a side effect of this change was to expose the shared getter `positionalTypes` through the analyzer's public API. Later, when the getter `sortedNamedTypes` was added to this class, that also was inadvertently exposed through the analyzer's public API. To correct that mistake, I've moved the reference to `SharedRecordTypeStructure` from the `implements` clause of `RecordType` to the `implements` clause of `RecordTypeImpl`. To avoid this change causing a breakage for clients, I've also added deprecated declarations of the getters `positionalTypes` and `sortedNamedTypes` to the `RecordType` class. This ensures that if any analyzer clients have already started depending on them, their code will continue to work, but they'll be alerted to the fact that the members will be removed in the future. In the process, I improved the types of `positionalTypes` and `sortedNamedTypes` so that they no longer refer to shared types; this should reduce further API exposure. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: If56b8689180d2214eccd7ec8ec4f4f10b31358f6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402620 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>

Change the analyzer's use of the following generic types so that it supplies the type parameter `InterfaceTypeImpl` instead of `InterfaceType` as the type it uses to represent interface types: - `MergedTypeConstraint` - `TypeAnalyzer` - `TypeAnalyzerOperations` - `TypeAnalyzerOperationsMixin` - `TypeConstraintFromArgument` - `TypeConstraintFromExtendsClause` - `TypeConstraintFromFunctionContext` - `TypeConstraintFromReturnType` - `TypeConstraintGenerator` - `TypeConstraintGeneratorMixin` - `TypeConstraintOrigin` - `TypeDeclarationMatchResult` - `UnknownTypeConstraintOrigin` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Id286f1f9ada4a0a516d8cb8e03b32caf10d626c7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403141 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

Change the analyzer's use of the following generic types so that it supplies the type parameter `StatementImpl` instead of `Statement` as the type of AST node it uses to represent statements: - `FlowAnalysis` - `TypeAnalyzerErrors` This required adjusting some `is` tests to test against concrete statement types instead of abstract public interface types (e.g. `is SwitchStatementImpl` instead of `is SwitchStatement`). In one place (in `FlowAnalysisHelper.getLabelTarget`) I changed an if-test to a switch statement so that I could use pattern matching to avoid type casts entirely. In some places, I was able to avoid adding casts by changing method parameters to accept concrete statement types instead of abstract public interface types. This required adding the `covariant` keyword to some methods. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I8910184ff71ccb97b52e5d44fbf0b6bddd6e273b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403180 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

Change the analyzer's use of the following generic types so that it supplies the type parameter `ExpressionImpl` instead of `Expression` as the type of AST node it uses to represent expressions: - `CaseHeadOrDefaultInfo` - `FlowAnalysis` - `MapPatternEntry` - `MatchContext` - `NullShortingMixin` - `PropertyTarget` - `SwitchExpressionMemberInfo` - `SwitchStatementMemberInfo` - `TypeAnalyzer` - `TypeAnalyzerErrors` This required adjusting some type casts and `is` tests to test against concrete expression types instead of abstract public interface types (e.g. `as SimpleIdentifierImpl` instead of `as SimpleIdentifier`). In some places I was able to avoid adding casts by changing method parameters to accept concrete statement types instead of abstract public interface types. This required adding the `covariant` keyword to some methods. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Ib33f0f5b3b30bfe2554316b5aa7c11524b63bcd3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403161 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

Change the analyzer's use of the following generic types so that it supplies the type parameter `DartPatternImpl` instead of `DartPattern` as the type of AST node it uses to represent patterns: - `MapPatternEntry` - `MatchContext` - `TypeAnalyzer` - `TypeAnalyzerErrors` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I3c8c3b1daf2dc6fbf03024d436e800fefc78bee4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403260 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

Change the analyzer's use of the following generic types so that it supplies the type parameter `AstNodeImpl` instead of `AstNode` as the type it uses to represent AST nodes: - `AssignedVariables` - `AssignedVariablesForTesting` - `CaseHeadOrDefaultInfo` - `FlowAnalysis` - `MatchContext` - `SwitchExpressionMemberInfo` - `SwitchStatementMemberInfo` - `TypeAnalyzer` - `TypeAnalyzerErrors` In some places I was able to avoid adding casts by changing method parameters to accept concrete AST node types instead of abstract public interface types. This required adding the `covariant` keyword to one method (`ResolverVisitor.visitTryStatement`). This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I98a6df704b64ff3efd3c73d05fb47409828fbf04 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403281 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The following shared getters are renamed so that their names are distinct from the corresponding getters in the analyzer: - `SharedFunctionTypeStructure.positionalParameterTypes` is renamed to `positionalParameterTypesShared` to be distinct from the analyzer's public getter `FunctionType.positionalParameterTypes`.* - `SharedFunctionTypeStructure.returnType` is renamed to `returnTypeShared` to be distinct from the analyzer's public getter `FunctionType.returnType`. - `SharedNamedFunctionParameterStructure.type` is renamed to `typeShared` to be distinct from the analyzer's public getter `FormalParameterElement.type`. - `SharedNamedTypeStructure.type` is renamed to `typeShared` to be distinct from the analyzer's public getter `RecordTypeNamedField.type`. - `SharedRecordTypeStructure.positionalTypes` is renamed to `positionalTypesShared` to be distinct from the analyzer's public getter `RecordType.positionalTypes`.* - `SharedRecordTypeStructure.sortedNamedTypes` is renamed to `sortedNamedTypesShared` to be distinct from the analyzer's public getter `RecordType.sortedNamedTypes`. - `SharedTypeParameterStructure.bound` is renamed to `boundShared` to be distinct from the analyzer's public getter `TypeParameterElement2.bound`. *Note that `FunctionType.positionalParameterTypes`, `RecordType.positionalTypes`, and `RecordType.sortedNamedTypes` were unintentionally exposed as part of the analyzer's public API. In a previous CL I marked them as deprecated. These renames pave the way for changing the analyzer's `DartType` class so that it implements `SharedTypeStructure<TypeImpl>` rather than `SharedTypeStructure<DartType>` (without the renames, the public getters mentioned above would all have to be changed to have type `TypeImpl`, and that in turn would expose `TypeImpl` through the analyzer public API, which we don't want to do). Once `DartType` implements `SharedTypeStructure<TypeImpl>`, that will allow all the other uses of `SharedTypeStructure<DartType>` in the analyzer to be gradually migrated to `SharedTypeStructure<TypeImpl>`. Once that is done, `DartType` can be changed so that it no longer implements `SharedTypeStructure<TypeImpl>` at all (`TypeImpl` will implement `SharedTypeStructure<TypeImpl> instead). This will free us up to make future changes to the `SharedTypeStructure` base class without inadvertently exposing those changes through the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I0686fdeae304f8948484516f0249841b79e7da6c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403625 Reviewed-by: Chloe Stefantsova <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

Now that the type of `ExecutableElementImpl._parameters` has been changed to `List<ParameterElementImpl>`, the type of `ExecutableElementImpl.formalParameters` can also be safely changed from `List<FormalParameterFragment>` to `List<ParameterElementImpl>`, and no longer requires a cast. (This works because `ParameterElementImpl` is a subtype of `FormalParameterFragment`.) Also, in `InheritanceManager3._topMerge`, it isn't necessary to use `toImpl()` to convert `resultType.parameters` to `List<ParameterElementImpl>`, because `resultType` is guaranteed to have been produced by `TypeSystemImpl.topMerge`. Instead, we can change `TypeSystemImpl.topMerge` so that when it produces a `FunctionType`, it always makes their parameter list a `List<ParameterElementImpl>`. (It was already the case that all the parameters in a `FunctionType` produced by `TypeSystemImpl.topMerge` are instances of `ParameterElementImpl`, so this is a straightforward change.) The change to `ExecutableElementImpl.formalParameters` will allow some types in various element model Impl classes to be changed from `DartType` to `TypeImpl`, which will in turn pave the way for changing the analyzer's `DartType` class so that it implements `SharedTypeStructure<TypeImpl>` rather than `SharedTypeStructure<DartType>`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I49153cebed389f549c22d41bebee1f9b85c173d0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404041 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

Change the methods in `TypeProviderImpl` so that their return types are all "Impl" types rather than analyzer public API types. This doesn't change the analyzer public API; it simply ensures that clients of `TypeProviderImpl` within the analyzer itself won't have to perform typecasts on the returned values in order to access the Impl classes. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API (See #59763). This change will reduce the number of type casts that have to be done in order to pass `TypeImpl` types to shared code rather than `DartType` types. Change-Id: Ia351c3e8893cf10671b04b813ee80ff2b3bda341 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402621 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

The type of `InterfaceTypeImpl.element` is changed from `InterfaceElement` to `InterfaceElementImpl`. This allows eliminating casts from a number of call sites, at the expense of adding a single cast to the `InterfaceTypeImpl` constructor. This cast is safe because all concrete implementations of `InterfaceElement` are subtypes of `InterfaceElementImpl`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I5d0ffa3b14788965589a57a3c061acac2f3c6930 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403942 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The types of the following getters are changed: - From `DartType` to `TypeImpl`: - `ExecutableElementImpl.returnType` - `FormalParameterElementImpl.type` - `FunctionTypeImpl.returnType` - `GenericFunctionTypeElementImpl.returnType` - `ParameterElementImpl_ofImplicitSetter.type` - `PropertyAccessorElementImpl_ImplicitGetter.returnType` - `PropertyAccessorElementImpl_ImplicitSetter.returnType` - `PropertyInducingElementImpl.type` - `RecordTypeFieldImpl.type` - `VariableElementImpl.type` - `VariableMember.type` - From `InterfaceType` to `InterfaceTypeImpl`: - `ConstructorElementImpl.returnType` - `ConstructorElementMixin.returnType` - `ConstructorMember.returnType` - `InterfaceElementImpl.thisType` - `InterfaceElementImpl2.thisType` - From `AugmentedInterfaceElement` to `InterfaceElementImpl2`: - `InterfaceElementImpl.augmented` There is no change to the analyzer public API. To make this possible, a few type casts had to be added, and the return type of some type inference methods had to be changed from `DartType` to `TypeImpl`. Some other type casts were able to be eliminated. I hope to eliminate the added type casts in future CLs. To ensure that type promotion continues to occur, a few `is` and `as` tests had to be changed so that they test against Impl types rather than public API types. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Ibf58396c46381aa49e465dcebc14877c6952aa01 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403943 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

Change the methods in `ElementsTypesMixin` so that their return types are all "Impl" types rather than analyzer public API types. This doesn't change the analyzer public API because `ElementsTypesMixin` is only used in tests internal to `package:analyzer`. This change allows `type_constraint_gatherer_test.dart` to represent all its types using `TypeImpl` rather than `DartType` without using type casts, which will in turn pave the way for switching the shared type constraint gathering code to using `TypeImpl` rather than `DartType`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API (See #59763). Change-Id: I9d15e9dd77f73271166b8e12de4d6f0a104cbb19 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403926 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The type `DartType` is changed so that instead of implementing `SharedTypeStructure<DartType>`, it implements `SharedTypeStructure<TypeImpl>`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. To ensure that this change doesn't expose the private `TypeImpl` type through the analyzer public API, an abstract override of `SharedTypeStructure.isStructurallyEqualTo` is added to `DartType`, with a parameter type of `DartType`. (Without the override, the parameter type would be `SharedTypeStructure<TypeImpl>`.) This method has been marked as deprecated, because it was not intentional for it to be exposed through the analyzer's public API. The following types, derived from `DartType`, require similar changes due to the fact that they implement shared types that are subtypes of `SharedTypeStructure`: - `DynamicTypeImpl` now implements `SharedDynamicTypeStructure<TypeImpl>`. - `FunctionTypeImpl` now implements `SharedFunctionTypeStructure<TypeImpl, TypeParameterElementImpl2, FormalParameterElementOrMember>`. - `InvalidTypeImpl` now implements `SharedInvalidTypeStructure<TypeImpl>`. - `NullTypeImpl` now implements `SharedNullTypeStructure<TypeImpl>`. - `RecordTypeImpl` now implements `SharedRecordTypeStructure<TypeImpl>`. - `VoidTypeImpl` now implements `SharedVoidTypeStructure<TypeImpl>`. - `UnknownInferredType` now implements `SharedUnknownTypeStructure<TypeImpl>`. Since the type `SharedNamedFunctionParameterStructure` is tightly integrated with `SharedFunctionTypeStructure`, `FormalParameterElementOrMember` must also be changed, so that it implements `SharedNamedFunctionParameterStructure<TypeImpl>` rather than `SharedNamedFunctionParameterStructure<DartType>`. Similarly, `TypeParameterElementImpl2` must be changed so that it implements `SharedTypeParameterStructure<TypeImpl>` rather than `SharedTypeParameterStructure<DartType>`. Follow-up CLs will change other uses of shared generic types so that they supply a type argument of `TypeImpl` rather than `DartType`. Change-Id: Ib36491056d46e4cd706c0dc2b85adc5314cb0b2c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403944 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

In the following methods in `TypeSystemImpl`, the return type is changed from `DartType` to `TypeImpl`: - `greatestClosure` - `greatestLowerBound` - `leastClosure` - `leastUpperBound` - `makeNullable` - `promoteToNonNull` This required adding a few casts to `TypeSystemImpl`, `LeastGreatestClosureHelper`, and `InterfaceTypeImpl`, which I believe I will be able to remove in future CLs. It also allowed removing some casts from `GreatestLowerBoundHelper`, `LeastUpperBoundHelper`, and `InterfaceTypeImpl`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I24f27090bb2d07ae33be9c2c8d7908ef71a96929 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404640 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The return types of `TypeImpl.asInstanceOf` and `TypeImpl.asInstanceOf2` are changed from `InterfaceType?` to `InterfaceTypeImpl?`. To reduce the number of casts that need to be added, the following changes are made in parallel: - The types of `TypeParameterTypeImpl.bound` and `TypeParameterTypeImpl.promotedBound` are changed to `TypeImpl`. - The type of `TypeParameterTypeImpl.element` is changed to `TypeParameterElementImpl`. - The type of `TypeParameterElementImpl.bound` is changed to `TypeImpl`. This allowed a null check and some type casts to be removed from methods in `FunctionTypeImpl` and `TypeParameterTypeImpl`. There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I65f84e1e27c20fcb320be4af0d131792d7396cce Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404720 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

The following methods of `TypeSystemImpl` have their return type changed from `DartType` to `TypeImpl`: - `flatten` - `greatestClosureOfSchema` - `leastClosureOfSchema` - `resolveToBound` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I9cf82a644b944f18bae9435ba68c896d45723281 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405062 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The type of `ExpressionImpl.staticType` is changed from `DartType?` to `TypeImpl?`, and the type of `ExpressionExtension.typeOrThrow` is changed from `DartType` to `TypeImpl`. To reduce the amount of casting required by these changes, a few other changes are included: - An additional extension `ExpressionImplExtension.typeOrThrow` is added; this has the same behavior as `ExpressionExtension.typeOrThrow`, but it doesn't require a type cast. - The type of `ErrorVerifier._typeProvider` is changed from `TypeProvider` to `TypeProviderImpl`. This allows calling `TypeProviderImpl` methods that are known to return `TypeImpl`. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I066262462bad32f4715e9a4b78b5d6697480c036 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405063 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

The following methods are changed so that they return `TypeImpl` rather than `DartType`: - `FreshTypeParameters.substitute` - `Substitution.substituteType` - `_NullSubstitution.substituteType` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Iaf4959cd7f6428d1dbe1b15089f7e2a30eff3495 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405080 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The signatures of `ExpressionImpl.resolveExpression` and all of its overrides are changed so that they expect a context which is a `TypeImpl` rather than a `DartType`. The signatures of expression `visit` methods in `ResolverVisitor` are changed in a similar way. To reduce the number of casts that this introduces, several fields and methods in the following classes have their types changed to use "Impl" types: - `ElementResolver` - `FunctionExpressionInvocationResolver` - `GenericFunctionInferenceTest` - `GenericInferrer` - `InstanceCreationExpressionResolver` - `InterfaceElementImpl` - `InterfaceElementImpl2` - `InterfacesMerger` - `InterfaceTypeImpl` - `InvocationInferenceHelper` - `InvocationInferrer` - `MethodElementImpl2` - `MethodInvocationResolver` - `MixinElementImpl` - `MixinElementImpl2` - `NamedTypeResolver` - `ResolutionReader` - `ResolverVisitor` - `Substitution` - `TypedLiteralResolver` - `TypeSystemImpl` - `_ClassInterfaceType` - `_LiteralResolution` - `_MixinInference` This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I11d476d712846c28b05e0fa1a5972fb7585e114a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405101 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The types of `TypeAnnotationImpl.type`, and the fields that override it, are changed from `DartType?` to `TypeImpl?`. Also, the type of `TypeAnnotationExtension.typeOrThrow` is changed from `DartType` to `TypeImpl`. To reduce the number of casts that need to be added, the following changes are made in parallel: - An additional extension `TypeAnnotationImplExtension.typeOrThrow` is added; this has the same behavior as `TypeAnnotationExtension.typeOrThrow`, but it doesn't require a type cast. - Some field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes: - `AstRewriter` - `EraseNonJSInteropTypes` - `ExtensionTypeErasure` - `FreshTypeParameters` - `FullInvocationInferrer` - `FunctionExpressionInvocationResolver` - `FunctionReferenceResolver` - `FunctionTypeBuilder` - `InstanceCreationInferrer` - `InvocationExpressionInferrer` - `InvocationInferrer` - `NamedTypeBuilder` - `NamedTypeResolver` - `ResolutionReader` - `TypeAliasElementImpl` - `TypeAliasElementImpl2` - `TypeImpl` - `TypeParameterElementImpl` - `TypeSystemImpl` There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I7f753508b53f6744677fd18f66858d70eb974093 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405221 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

The types of `LocalVariableElementImpl2.type=`, `PropertyInducingElementImpl.type=`, and `VariableElementImpl.type=` are all changed to accept `TypeImpl` rather than `DartType`. To reduce the number of casts that need to be added, some field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes: - `DeclarationBuilder` - `ElementFactory` - `ElementsTypesMixin` - `ForResolver` - `FunctionExpressionResolver` - `FunctionTypeBuilder` - `FunctionTypeTest` - `InstanceMemberInferrer` - `LocalVariableElementImpl2` - `NamedTypeBuilder` - `NonCovariantTypeParameterPositionVisitorTest` - `ReplacementVisitor` - `ResolutionReader` - `ResolutionVisitor` - `TypeBuilder` - `TypeReferencesAnyTest` - `TypesBuilder` - `_MockSdkElementsBuilder` - `_Node` There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I726bb1ce414eec3f360ae655c1f55e6be9e713fe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405242 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>

The types of the following members of `CollectionLiteralContext` are all changed to use `TypeImpl` rather than `DartType`: - `elementType` - `iterableType` - `keyType` - `valueType` To reduce the number of casts that need to be added, some field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes, mixins, and extensions: - `BodyInferenceContext` - `ElementsTypesMixin` - `ErrorDetectionHelpers` - `ExecutableMember` - `ExtensionMemberResolver` - `ExtensionsExtensions` - `ExtensionsExtensions2` - `FormalParameterElementMixin` - `FragmentedFunctionTypedElementMixin` - `FunctionTypeImpl` - `GenericFunctionInferenceTest` - `InstantiatedExtensionWithMember` - `InterfaceTypeImpl` - `InvocationInferrer` - `LeastUpperBoundHelper` - `LowerBoundTest` - `MethodMember` - `NotInstantiatedExtensionsExtensions` - `NotInstantiatedExtensionsExtensions2` - `ParameterElementExtension` - `ParameterElementMixin` - `ResolverVisitor` - `StringTypes` - `TypedLiteralResolver` - `TypeSystemImpl` - `TypeSystemOperations` - `_BoundsTestBase` - `_InferredCollectionElementTypeInformation` - `_NotInstantiatedExtension` - `_ParamInfo` There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I7a5f7c411e81b829be205f1919c933f8f874e5ad Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405404 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

The following methods of `InheritanceManager3` are changed so that their return types refer to Impl classes rather than analyzer public API classes: - `combineSignatures` - `getInherited2` - `getInheritedMap2` - `getMember` - `getMember2` To reduce the number of type casts this introduces, some of the parameters and return types of various private methods of `InheritanceManager3` are also changed to use Impl types, and some similar changes are made to the inheritance manager's internal data structures (`Interface` and `_ExtensionTypeCandidates`). Also, the following new classes are introduced, to act as common base classes for analyzer-internal classes in situations where no common base class previously existed: - `ExecutableElementOrMember` (the common base class for all internal classes that implement `ExecutableElement`) - `PropertyAccessorElementOrMember` (the common base class for all internal classes that implement `PropertyAccessorElement`) - `PropertyInducingElementOrMember` (the common base class for all internal classes that implement `PropertyInducingElement`) - `VariableElement2OrMember` (the common base class for all internal classes that implement `VariableElement2`) - `VariableElementOrMember` (the common base class for all internal classes that implement `VariableElement`) This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Ib7f20480c1f57af22a0214075e8f0e13c3429340 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405565 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

Field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes and extensions: - `CompoundAssignmentExpressionImpl` - `ExecutableElementExtensionQuestion` - `ExtensionElementImpl` - `ExtensionResolutionError` - `FunctionReferenceResolver` - `InstanceElementImpl2` - `InstantiatedExtensionWithMember` - `InstantiatedExtensionWithMember2` - `InterfaceElementImpl` - `InvocationInferrer` - `LexicalLookupResult` - `LocalVariableTypeProvider` - `PostfixExpressionResolver` - `PropertyElementResolverResult` - `RecordTypeExtension` - `ResolutionResult` - `ResolverVisitor` - `SimpleResolutionResult` - `TypePropertyResolver` There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I5d0d432301df38c7a5ccc7c65767cf73ce7b5d7d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405801 Commit-Queue: Paul Berry <[email protected]> Auto-Submit: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

Field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes: - `AssignmentExpressionResolver` - `DartPatternImpl` and its subtypes - `ExpressionImpl` and some of its subtypes - `PropertyElementResolver` - `RecordLiteralResolver` - `TypeParameterElementImpl2` - `TypeSystemImpl` - `_InferenceLogWriterImpl` There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: I2cc0dcf0a8e0d97053bf6da4975c729500bb1131 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405840 Auto-Submit: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>

Field types, getter types, method return types, and method parameter types are changed to `Impl` types in the following classes: - `BodyInferenceContext` - `BinaryExpressionResolver` - `ForResolver` - `ListPatternResolver` - `PrefixExpressionResolver` - `StaticTypeAnalyzer` - `TypeConstraintGatherer` - `TypeSystemOperations` - `YieldStatementResolver` Additionally, the type `DartType` is changed so that it no longer implements `SharedTypeStructure`. Instead, the private class `TypeImpl` implements `SharedTypeStructure`. There is no change to the analyzer public API. This is part of a larger arc of work to change the analyzer's use of the shared code so that the type parameters it supplies are not part of the analyzer public API. See #59763. Change-Id: Idbf09e25a26249a16e65a59274e66b477480d697 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405802 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>

stereotype441 · 2025-02-05T20:57:49Z

As of e2477ad, I believe nothing in _fe_analyzer_shared is being unintentionally exposed through the analyzer public API.

I would like to make a lint to make sure something similar never happens again. Tracking that work in #60058.

stereotype441 added legacy-area-analyzer Use area-devexp instead. analyzer-api Issues that impact the public API of the analyzer package labels Dec 19, 2024

stereotype441 self-assigned this Dec 19, 2024

srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Dec 27, 2024

keertip added the P2 A bug or feature request we're likely to work on label Dec 30, 2024

stereotype441 mentioned this issue Feb 5, 2025

Add custom lint to sanity check analyzer public API #60058

Closed

stereotype441 closed this as completed Feb 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shared interface members accidentally exposed through analyzer public API #59763

Shared interface members accidentally exposed through analyzer public API #59763

stereotype441 commented Dec 19, 2024

stereotype441 commented Feb 5, 2025

Shared interface members accidentally exposed through analyzer public API #59763

Shared interface members accidentally exposed through analyzer public API #59763

Comments

stereotype441 commented Dec 19, 2024

stereotype441 commented Feb 5, 2025