Skip to content
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

Nameless members on Flutter BlockBody docs #1367

Closed
Hixie opened this issue Apr 7, 2017 · 11 comments
Closed

Nameless members on Flutter BlockBody docs #1367

Hixie opened this issue Apr 7, 2017 · 11 comments
Assignees
Labels
customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@Hixie
Copy link
Contributor

Hixie commented Apr 7, 2017

Check out the Properties section of:
https://docs.flutter.io/flutter/widgets/BlockBody-class.html

@Hixie Hixie added customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures labels Apr 7, 2017
@jcollins-g
Copy link
Contributor

I have some asserts now that catch this case in checked mode and they blow up here among many other places in flutter docs, for reasons I don't understand yet.

@Hixie
Copy link
Contributor Author

Hixie commented Apr 7, 2017

Looks like a number of pages are affected by this. I just saw usability participant P3 go to a page that had half a dozen of these blank entries, and it looks embarrassing at best. :-)

@jcollins-g
Copy link
Contributor

This isn't on the early adopter's milestones but is among the first issues I intend to address once those are knocked out.

@jcollins-g jcollins-g modified the milestone: dartdoc 1.0 Apr 11, 2017
@jcollins-g
Copy link
Contributor

@bwilkerson

Started looking at this today and have a better idea what is going on here. There are apparently phantom elements in the analyzer tree generated under certain conditions which I'm still cataloging. One seems to be the use of assert in initializer lists, creating phantom FieldElements, and I've also seen it after the declaration of a private enum, creating a phantom TopLevelElement. These elements claim to be of type dynamic, with no name, and not synthetic.

Phantom FieldElements generate the symptom you're seeing. I'll work a bit more on establishing the signature of this (fortunately, element.nameOffset seems to be useful), and file an analyzer bug. In the meantime, I can also build a warning into dartdoc for when it sees them, but suppress the documenting of these phantom fields.

@jcollins-g
Copy link
Contributor

Learned more about why phantom elements are being generated; when actually generating docs, dartdoc is not obeying analysis options files. So, use of any language feature that requires an analysis option flag is likely to have poor dartdoc behavior (since the analyzer code it is depending on is misconfigured). This may well be why the asserts in initializer lists are triggering this bug. It seems likely that other language feature toggles are doing other subtly wrong things in dartdoc, too, so I'll try and get to the bottom of this.

It might also explain a lot of if (modelElement.name.isEmpty) continue; and similar things in dartdoc that I had difficulty pruning away entirely in #1368. I'll bet if analysis options are set properly the need for these will go away. Working on that now.

@jcollins-g jcollins-g self-assigned this Apr 13, 2017
@jcollins-g
Copy link
Contributor

@bwilkerson

This is going to be somewhat complicated. The modern analyzer generates new analysis contexts for every pubspec.yaml and .analysis_options file among other things in order to analyze code correctly. Dartdoc hardcodes one analysis context with the same options all the time, leading to this bug. I'm going to try to reuse the analyzer's ContextManager class to set up the analyzer in the same way the real one does to squish this and all similar bugs permanently. It already implements the necessary logic to be aware of all the option files, and, if I understand correctly, I really 'only' have to figure out how to glue it in here.

@scheglov
Copy link
Contributor

Of course it is better to use shared mechanism for creating analysis contexts, that supports all features for processing options.

Speaking specifically about using AnalysisContext in dartdoc - I'm not thrilled about this. AnalysisContext is dead, and we should start moving away from it ASAP, especially in projects that we are actively working on. It should be replaced with AnalysisDriver.

We should also move away from using Element.computeNode(), which I know dartdoc used to abuse. It might still do this, but it should really stop :-)

@jcollins-g
Copy link
Contributor

dartdoc still uses computeNode() pretty heavily so that's probably another issue.
WRT to AnalysisContext, dartdoc's use of analyzer is pretty straightforward so switching it to use AnalysisDriver shouldn't be that much harder I would think. I'll try that first, then.

@jcollins-g
Copy link
Contributor

flutter/engine#3575 causes further breakage in dartdoc for flutter because we aren't either using AnalysisContext or AnalysisDriver.

@jcollins-g
Copy link
Contributor

Some workarounds for this and flutter/flutter#9468 are coming up shortly that seem effective in solving both: forcing on enableAssertInitializer and manually reversing embedder links in dartdoc. PR coming up soon.

@jcollins-g
Copy link
Contributor

Head now has a workaround for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-flutter Issues originating from important to Flutter P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

3 participants