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

Problems with null check in NonParameterVariableElementImpl.enclosingElement3 #59750

Open
stereotype441 opened this issue Dec 17, 2024 · 1 comment
Labels
analyzer-api Issues that impact the public API of the analyzer package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@stereotype441
Copy link
Member

stereotype441 commented Dec 17, 2024

During my development of https://dart-review.googlesource.com/c/sdk/+/400660, I ran into a subtle bug where the following null check (in the class NonParameterVariableElementImpl) was failing:

  @override
  Element get enclosingElement3 => super.enclosingElement3!;

After discussion with @bwilkerson, I suspect this null check doesn't belong here. Variable elements can have a null enclosing element, if the variable is in a local function, as you can see from the declaration of _NonTopLevelVariableOrParameter.enclosingElement2:

  @override
  Element2? get enclosingElement2 {
    // TODO(dantup): Can we simplify this code and inline it into each class?

    var enclosingFunction = _enclosingFunction;
    return switch (enclosingFunction) {
      // There is no enclosingElement for a local function so we need to
      // determine whether our enclosing FunctionElementImpl is a local function
      // or not.
      // TODO(dantup): Is the real issue here that we're getting
      //  FunctionElementImpl here that should be LocalFunctionElementImpl?
      FunctionElementImpl()
          when enclosingFunction.enclosingElement3 is ExecutableElementImpl ||
              enclosingFunction.enclosingElement3 is VariableElementImpl =>
        null,
      // GenericFunctionTypeElementImpl currently implements Fragment but throws
      // if we try to access `element`.
      GenericFunctionTypeElementImpl() => null,
      // Otherwise, we have a valid enclosing element.
      Fragment(:var element) => element,
      _ => null,
    };
  }

This leads to some fragility, since local functions are fairly rare. I wonder if perhaps we should standardize on saying that either that (a) all local variables have a null enclosingElement2, or (b) all local variables have a non-null enclosingElement2.

To repro the failure, check out patchset 3 of https://dart-review.googlesource.com/c/sdk/+/400660 and change lines 1973-1974 of pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart from this:

          ..enclosingElement3 =
              first.firstFragment.enclosingFragment!.element.asElement!

to this:

          ..enclosingElement3 = first.enclosingElement2.asElement

Then run the test suite pkg/analyzer/test/src/dart/resolution/switch_expression_test.dart.

@stereotype441 stereotype441 added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-api Issues that impact the public API of the analyzer package labels Dec 17, 2024
@bwilkerson
Copy link
Member

I'll note that LabelElement2 has the same issue (using ! when the enclosing element can be null).

To make things worse, LocalFunctionElement always returns null, which is also wrong but in a different way.

@keertip keertip added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Dec 30, 2024
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-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants