Skip to content

Add scoped lookup to Libraries #2671

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 5 commits into from
Jun 9, 2021
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
16 changes: 14 additions & 2 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7506,6 +7506,18 @@ class _Renderer_Library extends RendererBase<Library> {
getters: _invisibleGetters['CommentReferable']));
},
),
'scope': Property(
getValue: (CT_ c) => c.scope,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(c, remainingNames, 'Scope'),
isNullValue: (CT_ c) => c.scope == null,
renderValue:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return renderSimple(c.scope, ast, r.template,
parent: r, getters: _invisibleGetters['Scope']);
},
),
'sdkLib': Property(
getValue: (CT_ c) => c.sdkLib,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down Expand Up @@ -11161,7 +11173,7 @@ class _Renderer_Package extends RendererBase<Package> {
}
}

String renderError(PackageTemplateData context, Template template) {
String renderIndex(PackageTemplateData context, Template template) {
return _render_PackageTemplateData(context, template.ast, template);
}

Expand Down Expand Up @@ -11361,7 +11373,7 @@ class _Renderer_PackageTemplateData extends RendererBase<PackageTemplateData> {
}
}

String renderIndex(PackageTemplateData context, Template template) {
String renderError(PackageTemplateData context, Template template) {
return _render_PackageTemplateData(context, template.ast, template);
}

Expand Down
13 changes: 11 additions & 2 deletions lib/src/model/comment_referable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,17 @@ mixin CommentReferable implements Nameable {
'PrefixElement detected, override [lookupViaScope] in subclass');
return null;
}
return recurseChildrenAndFilter(referenceLookup,
ModelElement.fromElement(resultElement, packageGraph), filter);
if (resultElement == null) return null;
var result = ModelElement.fromElement(resultElement, packageGraph);
if (result is Accessor) {
result = (result as Accessor).enclosingCombo;
}
if (result?.enclosingElement is Container) {
assert(false,
'[Container] member detected, override in subclass and handle inheritance');
return null;
}
return recurseChildrenAndFilter(referenceLookup, result, filter);
}

CommentReferable _lookupViaReferenceChildren(
Expand Down
4 changes: 1 addition & 3 deletions lib/src/model/documentable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ mixin MarkdownFileDocumentation implements Documentable, Canonicalization {
File get documentationFile;

@override
String get location => packageGraph.resourceProvider.pathContext
.toUri(documentationFile.path)
.toString();
String get location => '(${documentationFile.path})';
Copy link
Member

Choose a reason for hiding this comment

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

👍


@override
Set<String> get locationPieces => <String>{location};
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/dynamic.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Dynamic extends ModelElement {
ModelElement get canonicalModelElement => null;

@override
ModelElement get enclosingElement => throw UnsupportedError('');
ModelElement get enclosingElement => null;

/// And similarly, even if someone references it directly it can have
/// no hyperlink.
Expand Down
21 changes: 9 additions & 12 deletions lib/src/model/library.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:collection';

import 'package:analyzer/dart/ast/ast.dart' hide CommentReference;
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/scope.dart';
import 'package:analyzer/dart/element/type_system.dart';
import 'package:analyzer/dart/element/visitor.dart';
import 'package:analyzer/source/line_info.dart';
Expand Down Expand Up @@ -118,6 +119,10 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
CompilationUnitElement compilationUnit) =>
_getDefinedElements(compilationUnit);

/// Allow scope for Libraries.
@override
Scope get scope => element.scope;

List<String> __allOriginalModelElementNames;

/// Return true if this library is in a package configured to be treated as
Expand Down Expand Up @@ -577,7 +582,7 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
/*late final*/ HashMap<String, Set<ModelElement>> _modelElementsNameMap;

/// Map of [fullyQualifiedNameWithoutLibrary] to all matching [ModelElement]s
/// in this library. Used for code reference lookups.
/// in this library. Used for legacy code reference lookups.
HashMap<String, Set<ModelElement>> get modelElementsNameMap {
if (_modelElementsNameMap == null) {
_modelElementsNameMap = HashMap<String, Set<ModelElement>>();
Expand Down Expand Up @@ -656,18 +661,10 @@ class Library extends ModelElement with Categorization, TopLevelContainer {

Map<String, CommentReferable> _referenceChildren;
@override
// TODO(jcollins-g): This should take the import/export graph
// and resulting namespace into account.
Map<String, CommentReferable> get referenceChildren {
return _referenceChildren ??= {
for (var e in constants) e.name: e,
for (var e in enums) e.name: e,
for (var e in extensions) e.name: e,
for (var e in mixins) e.name: e,
for (var e in properties) e.name: e,
for (var e in typedefs) e.name: e,
for (var e in classes) e.name: e,
};
return _referenceChildren ??= Map.fromEntries(
element.exportNamespace.definedNames.entries.map((entry) => MapEntry(
entry.key, ModelElement.fromElement(entry.value, packageGraph))));
}

@override
Expand Down
7 changes: 1 addition & 6 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -494,12 +494,7 @@ class PackageGraph with CommentReferable, Nameable {
if (config.verboseWarnings && extendedDebug != null) {
messageParts.addAll(extendedDebug.map((s) => ' $s'));
}
String fullMessage;
if (messageParts.length <= 2) {
fullMessage = messageParts.join(', ');
} else {
fullMessage = messageParts.join('\n ');
}
var fullMessage = messageParts.join('\n ');

packageWarningCounter.addWarning(warnable, kind, message, fullMessage);
}
Expand Down
28 changes: 9 additions & 19 deletions test/end2end/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2217,16 +2217,12 @@ void main() {
MatchingLinkResult bothLookup(Warnable element, String codeRef) {
var originalLookupResult = originalLookup(element, codeRef);
var newLookupResult = newLookup(element, codeRef);
expect(newLookupResult, equals(originalLookupResult));
expect(newLookupResult.isEquivalentTo(originalLookupResult), isTrue);
return newLookupResult;
}

test('Verify basic linking inside class', () {
// parameter of [doAwesomeStuff]
// Parameter lookups are discarded with the original lookup code
expect(originalLookup(doAwesomeStuff, 'value'),
equals(MatchingLinkResult(null, warn: false)));
expect(newLookup(doAwesomeStuff, 'value'),
expect(bothLookup(doAwesomeStuff, 'value'),
equals(MatchingLinkResult(doAwesomeStuffParam)));

// Parent class of [doAwesomeStuff].
Expand All @@ -2240,37 +2236,31 @@ void main() {
equals(MatchingLinkResult(nameWithSingleUnderscore)));

// Top level class from [dart:core].
// TODO(jcollins-g): dart:core not recognized yet with new lookup code.
expect(originalLookup(doAwesomeStuff, 'String'),
expect(bothLookup(doAwesomeStuff, 'String'),
equals(MatchingLinkResult(string)));

// Another method in the same class.
expect(bothLookup(doAwesomeStuff, 'anotherMethod'),
equals(MatchingLinkResult(anotherMethod)));

// A top level function in this library.
// TODO(jcollins-g): top level functions not recognized yet with new lookup code.
expect(originalLookup(doAwesomeStuff, 'topLevelFunction'),
expect(bothLookup(doAwesomeStuff, 'topLevelFunction'),
equals(MatchingLinkResult(topLevelFunction)));

// A top level function in another library imported into this library.
// TODO(jcollins-g): namespace lookups are not yet implemented with new lookup code.
expect(originalLookup(doAwesomeStuff, 'function1'),
expect(bothLookup(doAwesomeStuff, 'function1'),
equals(MatchingLinkResult(function1)));

// A class in another library imported into this library.
// TODO(jcollins-g): namespace lookups are not yet implemented with new lookup code.
expect(originalLookup(doAwesomeStuff, 'Apple'),
expect(bothLookup(doAwesomeStuff, 'Apple'),
equals(MatchingLinkResult(Apple)));

// A top level constant in this library sharing the same name as a name in another library.
// TODO(jcollins-g): namespace lookups are not yet implemented with new lookup code.
expect(originalLookup(doAwesomeStuff, 'incorrectDocReference'),
expect(bothLookup(doAwesomeStuff, 'incorrectDocReference'),
equals(MatchingLinkResult(incorrectDocReference)));

// A top level constant in another library.
// TODO(jcollins-g): namespace lookups are not yet implemented with new lookup code.
expect(originalLookup(doAwesomeStuff, 'incorrectDocReferenceFromEx'),
expect(bothLookup(doAwesomeStuff, 'incorrectDocReferenceFromEx'),
equals(MatchingLinkResult(incorrectDocReferenceFromEx)));

// A prefixed constant in another library.
Expand All @@ -2284,7 +2274,7 @@ void main() {
equals(MatchingLinkResult(doesStuff)));

// A name of a class from an import of a library that exported that name.
expect(originalLookup(doAwesomeStuff, 'BaseClass'),
expect(bothLookup(doAwesomeStuff, 'BaseClass'),
equals(MatchingLinkResult(BaseClass)));

// A bracket operator within this class.
Expand Down
20 changes: 12 additions & 8 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,17 @@ void dartfmt() async {
// Filter out test packages as they always have strange formatting.
// Passing parameters to dartfmt for directories to search results in
// filenames being stripped of the dirname so we have to filter here.
void addFileToFix(String base, String fileName) {
void addFileToFix(String line) {
if (!line.startsWith('Changed ')) return;
var fileName = line.substring(8);
var pathComponents = path.split(fileName);
if (pathComponents.isNotEmpty && pathComponents.first == 'testing') {
return;
}
filesToFix.add(path.join(base, fileName));
filesToFix.add(fileName);
}

log('Validating dartfmt with version ${Platform.version}');
log('Validating dart format with version ${Platform.version}');
// TODO(jcollins-g): return to global once dartfmt can handle generic
// type aliases
for (var subDirectory in [
Expand All @@ -280,17 +282,19 @@ void dartfmt() async {
path.join('testing/test_package')
]) {
await SubprocessLauncher('dartfmt').runStreamed(
sdkBin('dartfmt'),
sdkBin('dart'),
[
'-n',
'format',
'-o',
'none',
subDirectory,
],
perLine: (n) => addFileToFix(subDirectory, n));
perLine: (n) => addFileToFix(n));
}
if (filesToFix.isNotEmpty) {
fail(
'dartfmt found files needing reformatting. Use this command to reformat:\n'
'dartfmt -w ${filesToFix.map((f) => "\'$f\'").join(' ')}');
'dart format found files needing reformatting. Use this command to reformat:\n'
'dart format ${filesToFix.map((f) => "\'$f\'").join(' ')}');
}
} else {
log('Skipping dartfmt check, requires latest dev version of SDK');
Expand Down