Skip to content

Add a list of potentially applicable extensions to every class page #2053

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 15 commits into from
Oct 31, 2019

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Oct 31, 2019

The beginnings of a fix for @munificent's #2021. We do not currently display methods that are applicable (that will need to come in a followup), but can calculate which extensions at a top level are likely, via the type system, to have methods that can be applied.

There are problems with this PR including inefficient lookups in the case where many extensions are defined and reaching into private analyzer interfaces, both of which will need to be fixed in followups.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Oct 31, 2019
@jcollins-g
Copy link
Contributor Author

Screenshots:

Screen Shot 2019-10-31 at 1 24 32 PM
Screen Shot 2019-10-31 at 1 25 06 PM
Screen Shot 2019-10-31 at 1 30 38 PM

Iterable<Extension> get potentiallyApplicableExtensions {
if (_potentiallyApplicableExtensions == null) {
if (name.contains('BaseTest')) {
print('hello');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to keep this debug code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, oops, will delete.

@@ -1326,6 +1350,28 @@ class Extension extends Container
ElementType.from(_extension.extendedType, library, packageGraph);
}

/// Returns [true] if this extension could be applicable to any possible
/// instantiation of [c].
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not sound mathematically rigorous :-)
Maybe "...if there is an instantiation of [c] to which this extension could be applied".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, better. Done.

}
return a;
}).toList(),
nullabilitySuffix: classInstantiated.nullabilitySuffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be explicit, use NullabilitySuffix.none.

Copy link
Contributor Author

@jcollins-g jcollins-g Oct 31, 2019

Choose a reason for hiding this comment

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

I would prefer to preserve whatever I was given rather than explicitly specify something potentially wrong.

@jcollins-g jcollins-g merged commit 75b3702 into master Oct 31, 2019
@jcollins-g jcollins-g deleted the type-tracking branch October 31, 2019 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants