Skip to content
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

Consider adding DynamicElement, NeverElement, DynamicFragment, and NeverFragment classes #60279

Open
stereotype441 opened this issue Mar 7, 2025 · 2 comments
Labels
analyzer-api Issues that impact the public API of the analyzer package area-devexp Developer experience items (DevTools, IDEs, analysis server, completions, refactorings, ...).

Comments

@stereotype441
Copy link
Member

As far as I can tell, all of the values in the pseudo-enum ElementKind (which is defined as part of element model 1 but also exposed via element model 2) have a corresponding Element2-derived class in the analyzer public API, except for:

  • CLASS_AUGMENTATION
  • COMPILATION_UNIT
  • DYNAMIC
  • EXPORT
  • IMPORT
  • NAME
  • NEVER
  • PART
  • RECORD
  • FUNCTION_TYPE_ALIAS
  • UNIVERSE

Of these 11, 4 are unused and should perhaps be removed (though it would technically be a breaking change to remove them, so perhaps they should simply be deprecated for now):

  • CLASS_AUGMENTATION
  • NAME
  • RECORD
  • UNIVERSE

A further 4 will be unused once element model 1 is removed because they aren't elements in element model 2:

  • COMPILATION_UNIT
  • EXPORT
  • IMPORT
  • PART

FUNCTION_TYPE_ALIAS will also be unused once element model 1 is removed, but that may be a bug (TypeAliasElementImpl, which is part of element model 1, has kind TYPE_ALIAS if nonfunction_type_aliases is enabled, and FUNCTION_TYPE_ALIAS otherwise; but TypeAliasElementImpl2, which is part of element model 2, always has kind TYPE_ALIAS)

Excluding all those, just 2 kinds remain:

  • DYNAMIC
  • NEVER

Both of these kinds have an associated Impl class (DynamicElementImpl2 and NeverElementImpl2), but no associated Element2-derived class in the analyzer public API.

As a result of this discrepancy, client code can match nearly every kind of Element2 using a simple object pattern (e.g. the pattern TypeParameterElement2 matches any type parameter element). But to match the elements for dynamic or Never, client code has to do something awkward like Element2(kind: ElementKind.DYNAMIC).

A similar issue applies to fragments.

I think we should consider adding API classes DynamicElement, NeverElement, DynamicFragment, and NeverFragment, so that the elements for dynamic and Never can be pattern-matched in the same way as all the other kinds of elements and fragments.

We should also consider adding visitDynamicElement and visitNeverElement methods to ElementVisitor2.

I discovered this issue while migrating Dart Kythe support to the new element model.

@stereotype441 stereotype441 added analyzer-api Issues that impact the public API of the analyzer package area-devexp Developer experience items (DevTools, IDEs, analysis server, completions, refactorings, ...). labels Mar 7, 2025
@scheglov
Copy link
Contributor

scheglov commented Mar 8, 2025

I think adding DynamicElement, NeverElement, DynamicFragment, and NeverFragment makes sense for consistency. IIRC by the same logic we added corresponding types to the API.

Another thought, probably related, that occurred to me, is that maybe ElementKind is not very useful.

@bwilkerson
Copy link
Member

I'm guessing that these would effectively be marker types and wouldn't actually have any behavior associated with them. Is that correct?

... maybe ElementKind is not very useful.

Maybe. I know that server currently uses it to compute a protocol-specific element kind for several operations. I don't know how consistently it's being used today (I seem to remember some special case logic in a couple of places, but don't remember why), but I do want to guard against unnecessary inconsistencies in the UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-api Issues that impact the public API of the analyzer package area-devexp Developer experience items (DevTools, IDEs, analysis server, completions, refactorings, ...).
Projects
None yet
Development

No branches or pull requests

3 participants