Implement this, operator, and parameter resolution on Containers (#2673)

jcollins-g · web-flow · commit 48f4706ac9be · 2021-06-11T12:11:51.000-07:00
* Start of prefix implementation...

* dartfmt

* fix this, operators, and parameters

* Fix error in test package

* rebuild

* Wrong negative case

* Update comment
diff --git a/lib/src/comment_references/parser.dart b/lib/src/comment_references/parser.dart
@@ -5,6 +5,69 @@
 import 'package:charcode/charcode.dart';
 import 'package:meta/meta.dart';
 
+const _operatorKeyword = 'operator';
+const Map<String, String> operatorNames = {
+  '[]': 'get',
+  '[]=': 'put',
+  '~': 'bitwise_negate',
+  '==': 'equals',
+  '-': 'minus',
+  '+': 'plus',
+  '*': 'multiply',
+  '/': 'divide',
+  '<': 'less',
+  '>': 'greater',
+  '>=': 'greater_equal',
+  '<=': 'less_equal',
+  '<<': 'shift_left',
+  '>>': 'shift_right',
+  '>>>': 'triple_shift',
+  '^': 'bitwise_exclusive_or',
+  'unary-': 'unary_minus',
+  '|': 'bitwise_or',
+  '&': 'bitwise_and',
+  '~/': 'truncate_divide',
+  '%': 'modulo'
+};
+
+class StringTrie {
+  final Map<int, StringTrie> children = {};
+
+  /// Does [this] node represent a valid entry in the trie?
+  bool valid = false;
+
+  /// Greedily match on the string trie.  Returns the index of the first
+  /// non-operator character if valid, otherwise -1.
+  int match(String toMatch, [int index = 0]) {
+    if (index < 0 || index >= toMatch.length) return valid ? index : 1;
+    var matchChar = toMatch.codeUnitAt(index);
+    if (children.containsKey(matchChar)) {
+      return children[matchChar].match(toMatch, index + 1);
+    }
+    return valid ? index : -1;
+  }
+
+  void addWord(String toAdd) {
+    var currentTrie = this;
+    for (var i in toAdd.codeUnits) {
+      currentTrie.children.putIfAbsent(i, () => StringTrie());
+      currentTrie = currentTrie.children[i];
+    }
+    currentTrie.valid = true;
+  }
+}
+
+StringTrie _operatorParseTrie;
+StringTrie get operatorParseTrie {
+  if (_operatorParseTrie == null) {
+    _operatorParseTrie = StringTrie();
+    for (var name in operatorNames.keys) {
+      _operatorParseTrie.addWord(name);
+    }
+  }
+  return _operatorParseTrie;
+}
+
 /// A parser for comment references.
 // TODO(jcollins-g): align with [CommentReference] from analyzer AST.
 class CommentReferenceParser {
@@ -30,7 +93,7 @@ class CommentReferenceParser {
   /// ```text
   ///   <rawCommentReference> ::= <prefix>?<commentReference><suffix>?
   ///
-  ///   <commentReference> ::= (<packageName> '.')? (<libraryName> '.')? <identifier> ('.' <identifier>)*
+  ///   <commentReference> ::= (<packageName> '.')? (<libraryName> '.')? <dartdocIdentifier> ('.' <identifier>)*
   /// ```
   List<CommentReferenceNode> _parseRawCommentReference() {
     var children = <CommentReferenceNode>[];
@@ -90,7 +153,7 @@ class CommentReferenceParser {
   ///
   /// <constructorPrefixHint> ::= 'new '
   ///
-  /// <leadingJunk> ::= ('const' | 'final' | 'var')(' '+)
+  /// <leadingJunk> ::= ('const' | 'final' | 'var' | 'operator')(' '+)
   /// ```
   _PrefixParseResult _parsePrefix() {
     if (_atEnd) {
@@ -108,25 +171,55 @@ class CommentReferenceParser {
     return _PrefixParseResult.missing;
   }
 
-  static const _whitespace = [$space, $tab, $lf, $cr];
+  static const _whitespace = {$space, $tab, $lf, $cr};
   static const _nonIdentifierChars = {
     $dot,
-    $lt,
     $gt,
     $lparen,
+    $lt,
     $rparen,
-    $slash,
     $backslash,
     $question,
     $exclamation,
     ..._whitespace,
   };
 
+  /// Advances the index forward to the end of the operator if one is
+  /// present and returns the operator's name.  Otherwise, leaves _index
+  /// unchanged and returns null.
+  String _tryParseOperator() {
+    var tryIndex = _index;
+    if (tryIndex + _operatorKeyword.length < codeRef.length &&
+        codeRef.substring(tryIndex, tryIndex + _operatorKeyword.length) ==
+            _operatorKeyword) {
+      tryIndex = tryIndex + _operatorKeyword.length;
+      while (_whitespace.contains(codeRef.codeUnitAt(tryIndex))) {
+        tryIndex++;
+      }
+    }
+
+    var result = operatorParseTrie.match(codeRef, tryIndex);
+    if (result == -1) {
+      return null;
+    }
+    _index = result;
+    return codeRef.substring(tryIndex, result);
+  }
+
+  /// Parse a dartdoc identifier.
+  ///
+  /// Dartdoc identifiers can include some operators.
   _IdentifierParseResult _parseIdentifier() {
     if (_atEnd) {
       return _IdentifierParseResult.endOfFile;
     }
     var startIndex = _index;
+
+    var foundOperator = _tryParseOperator();
+    if (foundOperator != null) {
+      return _IdentifierParseResult.ok(IdentifierNode(foundOperator));
+    }
+
     while (!_atEnd) {
       if (_nonIdentifierChars.contains(_thisChar)) {
         if (startIndex == _index) {
diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
@@ -3347,6 +3347,13 @@ class _Renderer_Container extends RendererBase<Container> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasInstanceFields == true,
                 ),
+                'hasParameters': Property(
+                  getValue: (CT_ c) => c.hasParameters,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasParameters == true,
+                ),
                 'hasPublicConstantFields': Property(
                   getValue: (CT_ c) => c.hasPublicConstantFields,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -3722,6 +3729,18 @@ class _Renderer_Container extends RendererBase<Container> {
                         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']);
+                  },
+                ),
                 'staticAccessors': Property(
                   getValue: (CT_ c) => c.staticAccessors,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -6415,6 +6434,13 @@ class _Renderer_GetterSetterCombo extends RendererBase<GetterSetterCombo> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasNoGetterSetter == true,
                 ),
+                'hasParameters': Property(
+                  getValue: (CT_ c) => c.hasParameters,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasParameters == true,
+                ),
                 'hasPublicGetter': Property(
                   getValue: (CT_ c) => c.hasPublicGetter,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -13679,6 +13705,13 @@ class _Renderer_TypeParameter extends RendererBase<TypeParameter> {
                         parent: r);
                   },
                 ),
+                'hasParameters': Property(
+                  getValue: (CT_ c) => c.hasParameters,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasParameters == true,
+                ),
                 'href': Property(
                   getValue: (CT_ c) => c.href,
                   renderVariable:
@@ -14996,6 +15029,7 @@ const _invisibleGetters = {
     'getterSetterDocumentationComment',
     'modelType',
     'isCallable',
+    'hasParameters',
     'parameters',
     'linkedParamsNoMetadata',
     'hasExplicitGetter',
diff --git a/lib/src/model/comment_referable.dart b/lib/src/model/comment_referable.dart
@@ -92,7 +92,7 @@ mixin CommentReferable implements Nameable {
     }
     if (result?.enclosingElement is Container) {
       assert(false,
-          '[Container] member detected, override in subclass and handle inheritance');
+          '[Container] member detected, support not implemented for analyzer scope inside containers');
       return null;
     }
     return recurseChildrenAndFilter(referenceLookup, result, filter);
diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/dart/element/scope.dart';
 import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/model_utils.dart' as model_utils;
@@ -32,6 +33,13 @@ abstract class Container extends ModelElement with TypeParameters {
   Container(Element element, Library library, PackageGraph packageGraph)
       : super(element, library, packageGraph);
 
+  // TODO(jcollins-g): Implement a ContainerScope that flattens supertypes?
+  @override
+  Scope get scope => null;
+
+  @override
+  bool get hasParameters => false;
+
   /// Is this a class (but not an enum)?
   bool get isClass =>
       element is ClassElement && !(element as ClassElement).isEnum;
@@ -251,9 +259,31 @@ abstract class Container extends ModelElement with TypeParameters {
   @override
   @mustCallSuper
   Map<String, CommentReferable> get referenceChildren {
-    return _referenceChildren ??= Map.fromEntries(allModelElements
-        .where((e) => e is! Accessor)
-        .map((e) => MapEntry(e.name, e)));
+    if (_referenceChildren == null) {
+      _referenceChildren = {};
+      for (var modelElement in allModelElements) {
+        if (modelElement is Accessor) continue;
+        if (modelElement is Operator) {
+          // TODO(jcollins-g): once todo in [Operator.name] is fixed, remove
+          // this special case.
+          _referenceChildren[modelElement.element.name] = modelElement;
+        } else {
+          _referenceChildren[modelElement.name] = modelElement;
+        }
+        // Don't complain about references to parameter names, but prefer
+        // referring to anything else.
+        // TODO(jcollins-g): Figure out something good to do in the ecosystem
+        // here to wean people off the habit of unscoped parameter references.
+        if (modelElement.hasParameters) {
+          for (var parameterElement in modelElement.parameters) {
+            _referenceChildren.putIfAbsent(
+                parameterElement.name, () => parameterElement);
+          }
+        }
+      }
+      _referenceChildren['this'] = this;
+    }
+    return _referenceChildren;
   }
 
   @override
diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart
@@ -195,6 +195,9 @@ mixin GetterSetterCombo on ModelElement {
   @override
   bool get isCallable => hasSetter;
 
+  @override
+  bool get hasParameters => hasSetter;
+
   @override
   List<Parameter> get parameters => setter.parameters;
 
diff --git a/lib/src/model/operator.dart b/lib/src/model/operator.dart
@@ -4,33 +4,10 @@
 
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/member.dart' show Member;
+import 'package:dartdoc/src/comment_references/parser.dart';
 import 'package:dartdoc/src/model/model.dart';
 
 class Operator extends Method {
-  static const Map<String, String> friendlyNames = {
-    '[]': 'get',
-    '[]=': 'put',
-    '~': 'bitwise_negate',
-    '==': 'equals',
-    '-': 'minus',
-    '+': 'plus',
-    '*': 'multiply',
-    '/': 'divide',
-    '<': 'less',
-    '>': 'greater',
-    '>=': 'greater_equal',
-    '<=': 'less_equal',
-    '<<': 'shift_left',
-    '>>': 'shift_right',
-    '>>>': 'triple_shift',
-    '^': 'bitwise_exclusive_or',
-    'unary-': 'unary_minus',
-    '|': 'bitwise_or',
-    '&': 'bitwise_and',
-    '~/': 'truncate_divide',
-    '%': 'modulo'
-  };
-
   Operator(MethodElement element, Library library, PackageGraph packageGraph)
       : super(element, library, packageGraph);
 
@@ -42,8 +19,8 @@ class Operator extends Method {
   @override
   String get fileName {
     var actualName = super.name;
-    if (friendlyNames.containsKey(actualName)) {
-      actualName = 'operator_${friendlyNames[actualName]}';
+    if (operatorNames.containsKey(actualName)) {
+      actualName = 'operator_${operatorNames[actualName]}';
     }
     return '$actualName.$fileType';
   }
@@ -57,6 +34,9 @@ class Operator extends Method {
 
   @override
   String get name {
+    // TODO(jcollins-g): New lookup code will no longer require this operator
+    // prefix.  Delete it and use super implementation after old lookup code
+    // is removed.
     return 'operator ${super.name}';
   }
 }
diff --git a/lib/src/model/type_parameter.dart b/lib/src/model/type_parameter.dart
@@ -47,6 +47,9 @@ class TypeParameter extends ModelElement {
     return _boundType;
   }
 
+  @override
+  bool get hasParameters => false;
+
   String _name;
 
   @override
diff --git a/test/comment_referable/parser_test.dart b/test/comment_referable/parser_test.dart
@@ -17,6 +17,9 @@ void main() {
     expect(hasHint, equals(constructorHint));
   }
 
+  void expectParsePassthrough(String codeRef) =>
+      expectParseEquivalent(codeRef, [codeRef]);
+
   void expectParseError(String codeRef) {
     expect(CommentReferenceParser(codeRef).parse(), isEmpty);
   }
@@ -43,11 +46,31 @@ void main() {
       expectParseEquivalent('this.is.valid(things)', ['this', 'is', 'valid']);
     });
 
+    test('Check that operator references parse', () {
+      expectParsePassthrough('[]');
+      expectParsePassthrough('<=');
+      expectParsePassthrough('>=');
+      expectParsePassthrough('>');
+      expectParsePassthrough('>>');
+      expectParsePassthrough('>>>');
+      expectParseEquivalent('operator []', ['[]']);
+      expectParseEquivalent('operator       []', ['[]']);
+      expectParseEquivalent('operator[]', ['[]']);
+      expectParseEquivalent('operator <=', ['<=']);
+      expectParseEquivalent('operator >=', ['>=']);
+
+      expectParseEquivalent('ThisThingy.operator []', ['ThisThingy', '[]']);
+      expectParseEquivalent('ThisThingy.operator [].parameter',
+          ['ThisThingy', '[]', 'parameter']);
+    });
+
     test('Basic negative tests', () {
       expectParseError(r'.');
       expectParseError(r'');
       expectParseError('foo(wefoi');
       expectParseError('<MoofMilker>');
+      expectParseError('>%');
+      expectParseError('>=>');
     });
   });
 }
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ mixin CommentReferable implements Nameable {`
`92`	`92`	`}`
`93`	`93`	`if (result?.enclosingElement is Container) {`
`94`	`94`	`assert(false,`
`95`		`- '[Container] member detected, override in subclass and handle inheritance');`
	`95`	`+ '[Container] member detected, support not implemented for analyzer scope inside containers');`
`96`	`96`	`return null;`
`97`	`97`	`}`
`98`	`98`	`return recurseChildrenAndFilter(referenceLookup, result, filter);`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,9 @@ class TypeParameter extends ModelElement {`
`47`	`47`	`return _boundType;`
`48`	`48`	`}`
`49`	`49`
	`50`	`+ @override`
	`51`	`+ bool get hasParameters => false;`
	`52`	`+`
`50`	`53`	`String _name;`
`51`	`54`
`52`	`55`	`@override`