Skip to content

Fix definingCombo == null for static field members of extension methods #2404

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

Merged
merged 4 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/src/generator/generator_frontend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ class GeneratorFrontEnd implements Generator {
_generatorBackend.generateProperty(
writer, packageGraph, lib, extension, property);
}

for (var staticField in filterNonDocumented(extension.staticFields)) {
indexAccumulator.add(staticField);
_generatorBackend.generateProperty(
writer, packageGraph, lib, extension, staticField);
}
}

for (var mixin in filterNonDocumented(lib.mixins)) {
Expand Down
8 changes: 2 additions & 6 deletions lib/src/model/accessor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,8 @@ class Accessor extends ModelElement implements EnclosedElement {
GetterSetterCombo get definingCombo {
if (_definingCombo == null) {
var variable = (element as PropertyAccessorElement).variable;
var accessor = isGetter ? variable.getter : variable.setter;
var accessorLibrary = Library(accessor.library, packageGraph);
var definingAccessor =
ModelElement.from(accessor, accessorLibrary, packageGraph)
as Accessor;
_definingCombo = definingAccessor.enclosingCombo;
_definingCombo = ModelElement.fromElement(variable, packageGraph);
assert(_definingCombo != null, 'Unable to find defining combo');
}
return _definingCombo;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,14 @@ abstract class ModelElement extends Canonicalization
}
}
if (e is PropertyAccessorElement) {
// TODO(jcollins-g): why test for ClassElement in enclosingElement?
// Accessors can be part of a [Container], or a part of a [Library].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha love the question on the left, the answer on the right.

if (e.enclosingElement is ClassElement ||
e.enclosingElement is ExtensionElement ||
e is MultiplyInheritedExecutableElement) {
if (enclosingContainer == null) {
return ContainerAccessor(e, library, packageGraph);
} else {
assert(e.enclosingElement is! ExtensionElement);
return ContainerAccessor.inherited(
e, library, packageGraph, enclosingContainer,
originalMember: originalMember);
Expand Down
12 changes: 10 additions & 2 deletions test/end2end/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1746,6 +1746,7 @@ void main() {
group('Extension', () {
Extension arm, leg, ext, fancyList, uphill;
Extension documentOnceReexportOne, documentOnceReexportTwo;
Extension staticFieldExtension;
Library reexportOneLib, reexportTwoLib;
Class apple,
anotherExtended,
Expand Down Expand Up @@ -1784,6 +1785,8 @@ void main() {
.firstWhere((e) => e.name == 'SimpleStringExtension')
.instanceMethods
.firstWhere((m) => m.name == 'doStuff');
staticFieldExtension = exLibrary.extensions
.firstWhere((e) => e.name == 'StaticFieldExtension');
extensions = exLibrary.publicExtensions.toList();
baseTest = fakeLibrary.classes.firstWhere((e) => e.name == 'BaseTest');
bigAnotherExtended =
Expand All @@ -1798,6 +1801,11 @@ void main() {
fakeLibrary.classes.firstWhere((e) => e.name == 'SuperMegaTron');
});

test('static fields inside extensions do not crash', () {
expect(staticFieldExtension.staticFields.length, equals(1));
expect(staticFieldExtension.staticFields.first.name, equals('aStatic'));
});

test('basic canonicalization for extensions', () {
expect(documentOnceReexportOne.isCanonical, isFalse);
expect(
Expand Down Expand Up @@ -1980,11 +1988,11 @@ void main() {
});

test('correctly finds all the extensions', () {
expect(exLibrary.extensions, hasLength(8));
expect(exLibrary.extensions, hasLength(9));
});

test('correctly finds all the public extensions', () {
expect(extensions, hasLength(6));
expect(extensions, hasLength(7));
});
});

Expand Down
4 changes: 4 additions & 0 deletions testing/test_package/lib/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,10 @@ extension on Object {
void bar() {}
}

extension StaticFieldExtension on Object {
static int aStatic;
}

/// This class has nothing to do with [_Shhh], [FancyList], or [AnExtension.call],
/// but should not crash because we referenced them.
/// We should be able to find [DocumentThisExtensionOnce], too.
Expand Down