Skip to content

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

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 9 commits into from
Jun 11, 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
103 changes: 98 additions & 5 deletions lib/src/comment_references/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind documenting this field? I think I know what it means, but maybe not in the context of a field on a whole trie.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


/// 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 {
Expand All @@ -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>[];
Expand Down Expand Up @@ -90,7 +153,7 @@ class CommentReferenceParser {
///
/// <constructorPrefixHint> ::= 'new '
///
/// <leadingJunk> ::= ('const' | 'final' | 'var')(' '+)
/// <leadingJunk> ::= ('const' | 'final' | 'var' | 'operator')(' '+)
/// ```
_PrefixParseResult _parsePrefix() {
if (_atEnd) {
Expand All @@ -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) {
Expand Down
34 changes: 34 additions & 0 deletions lib/src/generator/templates.runtime_renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -14996,6 +15029,7 @@ const _invisibleGetters = {
'getterSetterDocumentationComment',
'modelType',
'isCallable',
'hasParameters',
'parameters',
'linkedParamsNoMetadata',
'hasExplicitGetter',
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/comment_referable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 33 additions & 3 deletions lib/src/model/container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lib/src/model/getter_setter_combo.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
32 changes: 6 additions & 26 deletions lib/src/model/operator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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';
}
Expand All @@ -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}';
}
}
3 changes: 3 additions & 0 deletions lib/src/model/type_parameter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class TypeParameter extends ModelElement {
return _boundType;
}

@override
bool get hasParameters => false;

String _name;

@override
Expand Down
23 changes: 23 additions & 0 deletions test/comment_referable/parser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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('>=>');
});
});
}
Loading