Skip to content

Fix issue with generation of 'null' value enum fields #1470

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
Sep 11, 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
55 changes: 12 additions & 43 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ class Class extends ModelElement implements EnclosedElement {
..addAll(constructors)
..addAll(staticMethods)
..addAll(staticProperties)
..addAll(allInstanceMethods)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a duplicate, it was already addAll on line 454.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that line is not shown in the review :)

..addAll(_typeParameters);
}
return _allModelElements.toList();
Expand Down Expand Up @@ -694,7 +693,7 @@ class Class extends ModelElement implements EnclosedElement {
List<Field> get instanceProperties {
if (_instanceFields != null) return _instanceFields;
_instanceFields = _allFields
.where((f) => !f.isStatic && !f.isInherited)
.where((f) => !f.isStatic && !f.isInherited && !f.isConst)
.toList(growable: false)
..sort(byName);

Expand Down Expand Up @@ -1073,36 +1072,14 @@ abstract class EnclosedElement {
}

class Enum extends Class {
List<EnumField> _enumFields;

Enum(ClassElement element, Library library) : super(element, library);

@override
List<EnumField> get constants {
if (_enumFields != null) return _enumFields;

// This is a hack to give 'values' an index of -1 and all other fields
// their expected indices. https://github.com/dart-lang/dartdoc/issues/1176
var index = -1;

_enumFields = [];
for (FieldElement f in _cls.fields.where((f) => f.isConst)) {
// Enums do not have inheritance.
Accessor accessor = new ModelElement.from(f.getter, library);
EnumField enumField =
new ModelElement.from(f, library, index: index++, getter: accessor);
if (enumField.isPublic) _enumFields.add(enumField);
}
_enumFields.sort(byName);

return _enumFields;
}

@override
List<EnumField> get instanceProperties {
return super
.instanceProperties
.map((Field p) => new ModelElement.from(p.element, p.library))
.map((Field p) => new ModelElement.from(p.element, p.library,
getter: p.getter, setter: p.setter))
.toList(growable: false);
}

Expand Down Expand Up @@ -2150,13 +2127,8 @@ abstract class ModelElement extends Nameable
// parameter when given a null.
/// Do not construct any ModelElements unless they are from this constructor.
/// Specify enclosingClass only if this is to be an inherited object.
/// Specify index only if this is to be an EnumField.forConstant.
factory ModelElement.from(Element e, Library library,
{Class enclosingClass, int index, Accessor getter, Accessor setter}) {
// We don't need index in this key because it isn't a disambiguator.
// It isn't a disambiguator because EnumFields are not inherited, ever.
// TODO(jcollins-g): cleanup class hierarchy so that EnumFields aren't
// Inheritable, somehow?
{Class enclosingClass, Accessor getter, Accessor setter}) {
if (e is Member) {
e = Package.getBasestElement(e);
}
Expand Down Expand Up @@ -2193,21 +2165,18 @@ abstract class ModelElement extends Nameable
newModelElement = new Typedef(e, library);
}
if (e is FieldElement) {
assert(getter != null || setter != null);
if (enclosingClass == null) {
if (index != null) {
assert(getter != null);
newModelElement =
new EnumField.forConstant(index, e, library, getter);
if (e.isEnumConstant) {
int index = e.computeConstantValue().getField('index').toIntValue();
newModelElement = new EnumField.forConstant(index, e, library, getter);
} else if (e.enclosingElement.isEnum) {
newModelElement = new EnumField(e, library, getter, setter);
} else {
if (e.enclosingElement.isEnum) {
newModelElement = new EnumField(e, library, getter, setter);
} else {
assert(getter != null || setter != null);
newModelElement = new Field(e, library, getter, setter);
}
newModelElement = new Field(e, library, getter, setter);
}
} else {
assert(getter != null || setter != null);
// EnumFields can't be inherited, so this case is simpler.
newModelElement =
new Field.inherited(e, enclosingClass, library, getter, setter);
}
Expand Down
3 changes: 3 additions & 0 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,9 @@ void main() {

setUp(() {
animal = exLibrary.enums.firstWhere((e) => e.name == 'Animal');

/// Trigger code reference resolution
animal.documentationAsHtml;
});

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

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

/// Referencing [processMessage] (or other things) here should not break
/// enum constants ala #1445
enum Animal {
/// Single line docs.
CAT,
Expand Down
4 changes: 4 additions & 0 deletions testing/test_package_docs/ex/Animal-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ <h5>library ex</h5>

<div class="col-xs-12 col-sm-9 col-md-8 main-content">

<section class="desc markdown">
<p>Referencing <a href="ex/processMessage.html">processMessage</a> (or other things) here should not break
enum constants ala #1445</p>
</section>


<section class="summary offset-anchor" id="constants">
Expand Down
3 changes: 2 additions & 1 deletion testing/test_package_docs/ex/ex-library.html
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ <h2>Enums</h2>
<a href="ex/Animal-class.html">Animal</a>
</dt>
<dd>

Referencing <a href="ex/processMessage.html">processMessage</a> (or other things) here should not break
enum constants ala #1445
</dd>
</dl>
</section>
Expand Down