Prevent private or non-canonical elements from ever generating links #1589
Conversation
|
test added. |
gracias |
| String get href { | ||
| if (canonicalLibrary == null) return null; | ||
| return '${canonicalLibrary.dirName}/$fileName'; | ||
| if (!identical(canonicalModelElement, this)) |
There was a problem hiding this comment.
nit: braces for the if statement body
There was a problem hiding this comment.
Just for clarity, if this element is not the canonical one, we generate a link to the canonical one?
There was a problem hiding this comment.
Correct. While you could infer that from the code previously with all the "canonicalEnclosingElement" stuff, the modification here is intended to make that more clear and to make the object being linked to actually responsible for its own link generation.
Most of the broken link bugs I've seen either were attributable to bugs in how this worked, or were made harder to fix because of it (like this one). Now elements no longer point blindly into the filesystem hoping that the model really has a canonical element that matches, but we actually look up the element.
There was a problem hiding this comment.
I've got a second change on deck to eliminate the copypasta of hrefs everywhere; if possible I'd prefer to fix the braces there.
| if (canonicalLibrary == null) return null; | ||
| return '${canonicalLibrary.dirName}/${_constructor.enclosingElement.name}/$name.html'; | ||
| if (!identical(canonicalModelElement, this)) | ||
| return canonicalModelElement?.href; |
There was a problem hiding this comment.
here as well, and below
| expect(aStaticConstField.constantValue, '"A Constant Dog"'); | ||
| }); | ||
|
|
||
| test('privately constructed constants are unlinked', () { |
4 participants
Fixes #1588. First part of a long awaited refactor to rationalize how we calculate links, but moved up to fix a regression.