Skip to content

Fix function parameter references and empty href handling #2709

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
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
2 changes: 1 addition & 1 deletion lib/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ class Dartdoc {
var links = doc.querySelectorAll('a');
var stringLinks = links
.map((link) => link.attributes['href'])
.where((href) => href != null)
.where((href) => href != null && href != '')
.toList();

return Tuple2(stringLinks, baseHref);
Expand Down
13 changes: 3 additions & 10 deletions lib/src/model/comment_referable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,9 @@ extension on Scope {
/// [CommentReferable.referenceChildren] out of collections of other
/// [CommmentReferable]s.
extension CommentReferableEntryGenerators on Iterable<CommentReferable> {
/// Discards all entries that do not collide with [referable],
/// and generates an [MapEntry] using [referable]'s
/// [CommentReferable.referenceName] as a prefix for the item that does.
Iterable<MapEntry<String, CommentReferable>> onlyExplicitOnCollisionWith(
CommentReferable referable) =>
where((r) => r.referenceName == referable.referenceName).map(
(r) => MapEntry('${referable.referenceName}.${r.referenceName}', r));

/// Like [onlyExplicitOnCollisionWith], except does not discard non-conflicting
/// items.
/// Creates ordinary references except if there is a conflict with
/// [referable], it will generate a [MapEntry] using [referable]'s
/// [CommentReferable.referenceName] as a prefix for the conflicting item.
Iterable<MapEntry<String, CommentReferable>> explicitOnCollisionWith(
CommentReferable referable) =>
map((r) {
Expand Down
7 changes: 1 addition & 6 deletions lib/src/model/getter_setter_combo.dart
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,7 @@ mixin GetterSetterCombo on ModelElement {
if (_referenceChildren == null) {
_referenceChildren = {};
if (hasParameters) {
// Parameters in combos rarely matter for anything and are not part
// of the usual interface to a combo, so only reference them as part of
// [Container] fallbacks or if someone wants to explicitly specify a
// colliding parameter name.
_referenceChildren
.addEntries(parameters.onlyExplicitOnCollisionWith(this));
_referenceChildren.addEntries(parameters.explicitOnCollisionWith(this));
}
_referenceChildren
.addEntries(modelType.typeArguments.explicitOnCollisionWith(this));
Expand Down
16 changes: 7 additions & 9 deletions lib/src/model/parameter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class Parameter extends ModelElement implements EnclosedElement {
ParameterElement element, Library library, PackageGraph packageGraph,
{ParameterMember originalMember})
: super(element, library, packageGraph, originalMember);

String get defaultValue {
if (!hasDefaultValue) return null;
return element.defaultValueCode;
Expand Down Expand Up @@ -85,17 +84,16 @@ class Parameter extends ModelElement implements EnclosedElement {
Map<String, CommentReferable> get referenceChildren {
if (_referenceChildren == null) {
_referenceChildren = {};
if (isCallable) {
_referenceChildren
.addEntriesIfAbsent(parameters.explicitOnCollisionWith(this));
var _modelType = modelType;
if (_modelType is Callable) {
_referenceChildren.addEntriesIfAbsent(
_modelType.parameters.explicitOnCollisionWith(this));
}
_referenceChildren.addEntriesIfAbsent(
modelType.typeArguments.explicitOnCollisionWith(this));
if (modelType is Callable) {
_referenceChildren.addEntriesIfAbsent((modelType as Callable)
.returnType
.typeArguments
.explicitOnCollisionWith(this));
if (_modelType is Callable) {
_referenceChildren.addEntriesIfAbsent(
_modelType.returnType.typeArguments.explicitOnCollisionWith(this));
}
}
return _referenceChildren;
Expand Down
27 changes: 26 additions & 1 deletion test/end2end/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2363,7 +2363,8 @@ void main() {
function1,
topLevelFunction,
aFunctionUsingRenamedLib;
TopLevelVariable incorrectDocReference,
TopLevelVariable aSetterWithFunctionParameter,
incorrectDocReference,
incorrectDocReferenceFromEx,
nameWithTwoUnderscores,
nameWithSingleUnderscore,
Expand Down Expand Up @@ -2392,6 +2393,7 @@ void main() {
redHerring,
yetAnotherName,
somethingShadowyParameter;
Parameter fParam, fParamA, fParamB, fParamC;
Field forInheriting,
action,
initializeMe,
Expand Down Expand Up @@ -2515,9 +2517,32 @@ void main() {
.firstWhere((m) => m.name == 'aMethod');
yetAnotherName =
aMethod.allParameters.firstWhere((p) => p.name == 'yetAnotherName');

aSetterWithFunctionParameter = fakeLibrary.properties
.firstWhere((p) => p.name == 'aSetterWithFunctionParameter');
fParam = aSetterWithFunctionParameter.parameters
.firstWhere((p) => p.name == 'fParam');
fParamA = (fParam.modelType as Callable)
.parameters
.firstWhere((p) => p.name == 'fParamA');
fParamB = (fParam.modelType as Callable)
.parameters
.firstWhere((p) => p.name == 'fParamB');
fParamC = (fParam.modelType as Callable)
.parameters
.firstWhere((p) => p.name == 'fParamC');
});

group('Parameter references work properly', () {
test('via a setter with a function parameter', () {
expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamA'),
equals(MatchingLinkResult(fParamA)));
expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamB'),
equals(MatchingLinkResult(fParamB)));
expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamC'),
equals(MatchingLinkResult(fParamC)));
});

test('in class scope overridden by fields', () {
expect(bothLookup(FactoryConstructorThings, 'aName'),
equals(MatchingLinkResult(aNameField)));
Expand Down
6 changes: 5 additions & 1 deletion testing/test_package/lib/fake.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1306,4 +1306,8 @@ class TypeParameterThingsExtendedQ extends TypeParameterThings<String, FactoryCo
@override
/// I override documentation so I can reference [QTypeParam].
FactoryConstructorThings aMethod<QTypeParam>(String aParam, QTypeParam anotherParam) => null;
}
}

/// We should still be able to reference [fParam.fParamA], [fParam.fParamB],
/// and [fParam.fParamC].
void set aSetterWithFunctionParameter(bool fParam(int fParamA, String fParamB, String fParamC)) {}