From 2fa0919b03f41851faebce6619b1ef4c528db917 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 8 Jun 2021 16:35:13 -0700 Subject: [PATCH 1/4] Add scoped lookup to Libraries --- .../templates.runtime_renderers.dart | 11 ++++++++++ lib/src/model/comment_referable.dart | 13 +++++++++-- lib/src/model/documentable.dart | 4 +--- lib/src/model/dynamic.dart | 2 +- lib/src/model/library.dart | 22 +++++++++---------- lib/src/model/package_graph.dart | 7 +----- test/end2end/model_test.dart | 4 ++-- tool/grind.dart | 20 ++++++++++------- 8 files changed, 49 insertions(+), 34 deletions(-) diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index f524624927..f098355209 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -7443,6 +7443,17 @@ class _Renderer_Library extends RendererBase { (e) => renderSimple(e, ast, r.template, parent: r)); }, ), + 'scope': Property( + getValue: (CT_ c) => c.scope, + renderVariable: (CT_ c, Property self, + List remainingNames) => + self.renderSimpleVariable(c, remainingNames, 'Scope'), + isNullValue: (CT_ c) => c.scope == null, + renderValue: + (CT_ c, RendererBase r, List ast) { + return renderSimple(c.scope, ast, r.template, parent: r); + }, + ), 'sdkLib': Property( getValue: (CT_ c) => c.sdkLib, renderVariable: (CT_ c, Property self, diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart index 91afd2e875..fb62e390b8 100644 --- a/lib/src/model/comment_referable.dart +++ b/lib/src/model/comment_referable.dart @@ -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( diff --git a/lib/src/model/documentable.dart b/lib/src/model/documentable.dart index e5bf497653..4779b0bda6 100644 --- a/lib/src/model/documentable.dart +++ b/lib/src/model/documentable.dart @@ -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})'; @override Set get locationPieces => {location}; diff --git a/lib/src/model/dynamic.dart b/lib/src/model/dynamic.dart index 49e109834e..5e49d673a0 100644 --- a/lib/src/model/dynamic.dart +++ b/lib/src/model/dynamic.dart @@ -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. diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index f6b7fb62c7..3c17641d38 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -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'; @@ -118,6 +119,11 @@ class Library extends ModelElement with Categorization, TopLevelContainer { CompilationUnitElement compilationUnit) => _getDefinedElements(compilationUnit); + @override + + /// Allow scope for Libraries. + Scope get scope => element.scope; + List __allOriginalModelElementNames; /// Return true if this library is in a package configured to be treated as @@ -577,7 +583,7 @@ class Library extends ModelElement with Categorization, TopLevelContainer { /*late final*/ HashMap> _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> get modelElementsNameMap { if (_modelElementsNameMap == null) { _modelElementsNameMap = HashMap>(); @@ -656,18 +662,10 @@ class Library extends ModelElement with Categorization, TopLevelContainer { Map _referenceChildren; @override - // TODO(jcollins-g): This should take the import/export graph - // and resulting namespace into account. Map 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 diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index a805b80fad..fed4c27b9f 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -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); } diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 89cd9bf0d3..160daf48cf 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -2217,7 +2217,7 @@ 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; } @@ -2241,7 +2241,7 @@ void main() { // 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. diff --git a/tool/grind.dart b/tool/grind.dart index 330940e965..b49477bed8 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -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 [ @@ -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'); From bc3d07e31006fb77896079db564b97ef0e39b220 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 8 Jun 2021 16:40:06 -0700 Subject: [PATCH 2/4] Place @override adjacent to scope impl --- lib/src/model/library.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index 3c17641d38..cc153022c0 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -119,9 +119,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer { CompilationUnitElement compilationUnit) => _getDefinedElements(compilationUnit); - @override - /// Allow scope for Libraries. + @override Scope get scope => element.scope; List __allOriginalModelElementNames; From 798a881a48c3509a4bf95157fab6c51c5d317eef Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 8 Jun 2021 16:57:09 -0700 Subject: [PATCH 3/4] Enable more cases, yay --- test/end2end/model_test.dart | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 160daf48cf..753bd85430 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -2222,11 +2222,7 @@ void main() { } 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]. @@ -2240,7 +2236,6 @@ void main() { equals(MatchingLinkResult(nameWithSingleUnderscore))); // Top level class from [dart:core]. - // TODO(jcollins-g): dart:core not recognized yet with new lookup code. expect(bothLookup(doAwesomeStuff, 'String'), equals(MatchingLinkResult(string))); @@ -2249,28 +2244,23 @@ void main() { 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. @@ -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. From bd8d3c5bc69168655d90cb5f4147d10e41296931 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 8 Jun 2021 17:12:41 -0700 Subject: [PATCH 4/4] post merge rebuild --- lib/src/generator/templates.runtime_renderers.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index ec736b3618..ad8cc1a1ba 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -7514,7 +7514,8 @@ class _Renderer_Library extends RendererBase { isNullValue: (CT_ c) => c.scope == null, renderValue: (CT_ c, RendererBase r, List ast) { - return renderSimple(c.scope, ast, r.template, parent: r); + return renderSimple(c.scope, ast, r.template, + parent: r, getters: _invisibleGetters['Scope']); }, ), 'sdkLib': Property( @@ -11172,7 +11173,7 @@ class _Renderer_Package extends RendererBase { } } -String renderError(PackageTemplateData context, Template template) { +String renderIndex(PackageTemplateData context, Template template) { return _render_PackageTemplateData(context, template.ast, template); } @@ -11372,7 +11373,7 @@ class _Renderer_PackageTemplateData extends RendererBase { } } -String renderIndex(PackageTemplateData context, Template template) { +String renderError(PackageTemplateData context, Template template) { return _render_PackageTemplateData(context, template.ast, template); }