Skip to content

Fix truncation in middle of links for constants #1544

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 3 commits into from
Nov 28, 2017
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
4 changes: 1 addition & 3 deletions lib/src/element_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ class ElementType {
} else {
typeArguments = type.typeFormals.map((f) => f.type);
}
return typeArguments
.map(_getElementTypeFrom)
.toList();
return typeArguments.map(_getElementTypeFrom).toList();
} else {
return (_type as ParameterizedType)
.typeArguments
Expand Down
69 changes: 34 additions & 35 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ class Accessor extends ModelElement

/// Call exactly once to set the enclosing combo for this Accessor.
set enclosingCombo(GetterSetterCombo combo) {
assert(_enclosingCombo == null || combo == _enclosingCombo);
assert(combo != null);
_enclosingCombo = combo;
assert(_enclosingCombo == null || combo == _enclosingCombo);
assert(combo != null);
_enclosingCombo = combo;
}

bool get isSynthetic => element.isSynthetic;
Expand All @@ -274,7 +274,8 @@ class Accessor extends ModelElement
@override
String get computeDocumentationComment {
if (isSynthetic) {
String docComment = (element as PropertyAccessorElement).variable.documentationComment;
String docComment =
(element as PropertyAccessorElement).variable.documentationComment;
// If we're a setter, only display something if we have something different than the getter.
// TODO(jcollins-g): modify analyzer to do this itself?
if (isGetter ||
Expand Down Expand Up @@ -1204,7 +1205,7 @@ class EnumField extends Field {
: super(element, library, getter, null);

@override
String get constantValue {
String get constantValueBase {
if (name == 'values') {
return 'const List<${_field.enclosingElement.name}>';
} else {
Expand Down Expand Up @@ -1260,7 +1261,6 @@ class EnumField extends Field {
class Field extends ModelElement
with GetterSetterCombo, Inheritable, SourceCodeMixin
implements EnclosedElement {
String _constantValue;
bool _isInherited = false;
Class _enclosingClass;
@override
Expand Down Expand Up @@ -1296,20 +1296,6 @@ class Field extends ModelElement
return super.documentation;
}

String get constantValue {
if (_constantValue != null) return _constantValue;

if (_field.computeNode() == null) return null;
var v = _field.computeNode().toSource();
if (v == null) return null;
var string = v.substring(v.indexOf('=') + 1, v.length).trim();
_constantValue = string.replaceAll("${modelType.name}", "${modelType.linkedName}");

return _constantValue;
}

String get constantValueTruncated => truncateString(constantValue, 200);

@override
ModelElement get enclosingElement {
if (_enclosingClass == null) {
Expand Down Expand Up @@ -1441,11 +1427,34 @@ abstract class GetterSetterCombo implements ModelElement {
return allFeatures;
}


@override
ModelElement enclosingElement;
bool get isInherited;

String _constantValueBase;
String get constantValueBase {
if (_constantValueBase == null) {
if (element.computeNode() != null) {
var v = element.computeNode().toSource();
if (v == null) return null;
var string = v.substring(v.indexOf('=') + 1, v.length).trim();
_constantValueBase =
const HtmlEscape(HtmlEscapeMode.UNKNOWN).convert(string);
}
}
return _constantValueBase;
}

String linkifyWithModelType(String text) {
RegExp r = new RegExp("\\b${modelType.name}\\b");
Copy link
Member

Choose a reason for hiding this comment

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

Is this called frequently? If so, we may want to cache the regexp in an instance field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is not called very often per GetterSetterCombo (maybe twice?). Probably not worth it at this point.

return text?.replaceAll(r, modelType.linkedName);
}

String get constantValue => linkifyWithModelType(constantValueBase);

String get constantValueTruncated =>
linkifyWithModelType(truncateString(constantValueBase, 200));

/// Returns true if both accessors are synthetic.
bool get hasSyntheticAccessors {
if ((hasPublicGetter && getter.element.isSynthetic) ||
Expand Down Expand Up @@ -2868,16 +2877,16 @@ abstract class ModelElement extends Nameable
Library lib;
// TODO(jcollins-g): get rid of dynamic special casing
if (element.kind != ElementKind.DYNAMIC) {
lib = _findOrCreateEnclosingLibraryFor((element as dynamic).type.element);
lib = _findOrCreateEnclosingLibraryFor(
(element as dynamic).type.element);
}
_modelType = new ElementType(
(element as dynamic).type, new ModelElement.from((element as dynamic).type.element, lib));
_modelType = new ElementType((element as dynamic).type,
new ModelElement.from((element as dynamic).type.element, lib));
}
}
return _modelType;
}


@override
String get name => element.name;

Expand Down Expand Up @@ -4402,16 +4411,6 @@ class TopLevelVariable extends ModelElement
@override
bool get isInherited => false;

String get constantValue {
var v = _variable.computeNode().toSource();
if (v == null) return '';
var string = v.substring(v.indexOf('=') + 1, v.length).trim();
string = HTML_ESCAPE.convert(string);
return string.replaceAll(modelType.name, modelType.linkedName);
}

String get constantValueTruncated => truncateString(constantValue, 200);

@override
String get documentation {
// Verify that hasSetter and hasGetterNoSetter are mutually exclusive,
Expand Down
7 changes: 4 additions & 3 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ String stripComments(String str) {

String truncateString(String str, int length) {
if (str != null && str.length > length) {
return str.substring(0, length) + '…';
} else {
return str;
// Do not call this on unsanitized HTML.
assert(!str.contains("<"));
return '${str.substring(0, length)}…';
}
return str;
}

String pluralize(String word, int count) => count == 1 ? word : '${word}s';
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,4 @@ packages:
source: hosted
version: "2.1.13"
sdks:
dart: ">=1.23.0 <=2.0.0-dev.7.0"
dart: ">=1.23.0 <=2.0.0-dev.8.0"
17 changes: 16 additions & 1 deletion test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ void main() {
});

test('correctly finds all the classes', () {
expect(classes, hasLength(22));
expect(classes, hasLength(24));
});

test('abstract', () {
Expand Down Expand Up @@ -1711,6 +1711,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
prettyColorsConstant,
deprecated;

Field aStaticConstField, aName;

setUp(() {
greenConstant =
exLibrary.constants.firstWhere((c) => c.name == 'COLOR_GREEN');
Expand All @@ -1721,6 +1723,19 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
cat = exLibrary.constants.firstWhere((c) => c.name == 'MY_CAT');
deprecated =
exLibrary.constants.firstWhere((c) => c.name == 'deprecated');
Class Dog = exLibrary.allClasses.firstWhere((c) => c.name == 'Dog');
aStaticConstField =
Dog.allFields.firstWhere((f) => f.name == 'aStaticConstField');
aName = Dog.allFields.firstWhere((f) => f.name == 'aName');
});

test('substrings of the constant values type are not linked (#1535)', () {
expect(aName.constantValue,
'const ExtendedShortName(&quot;hello there&quot;)');
});

test('constant field values are escaped', () {
expect(aStaticConstField.constantValue, '&quot;A Constant Dog&quot;');
});

test('has a fully qualified name', () {
Expand Down
12 changes: 12 additions & 0 deletions testing/test_package/lib/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ typedef String processMessage<T>(String msg);

typedef String ParameterizedTypedef<T>(T msg, int foo);

class ShortName {
final String aParameter;
const ShortName(this.aParameter);
}

class ExtendedShortName extends ShortName {
const ExtendedShortName(String aParameter) : super(aParameter);
}

/// Referencing [processMessage] (or other things) here should not break
/// enum constants ala #1445
enum Animal {
Expand Down Expand Up @@ -246,6 +255,9 @@ class Dog implements Cat, E {
final int aFinalField;
static const String aStaticConstField = "A Constant Dog";

/// Verify link substitution in constants (#1535)
static const ShortName aName = const ExtendedShortName("hello there");

@protected
final int aProtectedFinalField;

Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/Animal-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/Apple-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/B-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/COLOR-constant.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/COLOR_GREEN-constant.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/COLOR_ORANGE-constant.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/COMPLEX_COLOR-constant.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/Cat-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/CatString-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/ConstantCat-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ <h5>library ex</h5>
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
<li><a href="ex/Dog-class.html">Dog</a></li>
<li><a href="ex/E-class.html">E</a></li>
<li><a href="ex/ExtendedShortName-class.html">ExtendedShortName</a></li>
<li><a href="ex/F-class.html">F</a></li>
<li><a href="ex/ForAnnotation-class.html">ForAnnotation</a></li>
<li><a href="ex/HasAnnotation-class.html">HasAnnotation</a></li>
Expand All @@ -57,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassExtendsPrivateClass-class.html">PublicClassExtendsPrivateClass</a></li>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/ShortName-class.html">ShortName</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
Expand Down
Loading