This repository was archived by the owner on Jun 20, 2019. It is now read-only.
Now ready: Priority angular analysis #273
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bwilkerson
approved these changes
Apr 11, 2017
mk13
approved these changes
Apr 11, 2017
I think before we were getting a last-one-wins effect, where the last analyzed dart/html pair would cover up all the errors for every other dart/html pair. As such, I need to carefully test that this change doesn't introduce new false errors (from them being covered up before). But now with this, we can report html errors back for multiple contexts from a single method when that's requested. Next I just have to use that method on getErrors when we have multiple relationships tracked already.
MichaelRFairhurst
changed the title
|
Ping @mk13 for re-review of latest changes |
mk13
reviewed
Apr 14, 2017
| // Try resolving HTML using the existing dart/html relationships which may | ||
| // be already known. However, if we don't see any relationships, try using | ||
| // the .dart equivalent. Better than no result -- the real one WILL come. | ||
| if (_fileTracker.getDartPathsReferencingHtml(path) == 0) { |
There was a problem hiding this comment.
Doesn't this function return a List? Did you mean .length?
There was a problem hiding this comment.
Yes, great catch!
mk13
added a commit
that referenced
this pull request
May 3, 2017
* Sync to origin master * checkpoint * Initial ast integration - ranges not behaving properly * Minor bug fix * checkpoint * Tasks tests all passing * Checkpoint before error code fix * Tasks tests set back to 100% after introducing error codes * resolver_test midway checkpoint * Checkpoint * Full test coverage in analyzer. Server still left * All tests passing * Make paths explicit * Remove comments * Attributes now sorted by offset. Improves performance in auto completion * DOCTYPE now acceptable * Created separate Ast for top level document container * Refactored primary function in completion.dart * Minor code clean up * Fuzz tester bug fix * Revert try catch block * Rename exception codes after changes in angular_ast * Bug fix on [class. [style. [attr. bindings; range error * Allow spaces in msutaches * Autocompletion in text attribute value is now empty * Allow for plain [class] binding * Fixed update_deps.sh to work with angular_ast * Clean up deps and remove some unnecessary comments * Changed pubspec to pull from master * Defend against files hidden by generated files (#271) * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Travis sdk fix (#272) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * First round of quick fixes based on feedback * Tick to 8 (#277) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Tick to 0.0.8 * remove comment * Error codes no longer use hard offsets; uses string instead * Track @Attribute annotated constructor parameters (#270) * Merged attribute generating logic to be agnostic of Element or template type. * Assert for indexOf added into test. Moved comments for clarity * Remove comment-blocked chunk * Resolve plain attributes -- in ranges, and strchecks for inputs. (#278) * Resolve plain attributes -- in ranges, and strchecks for inputs. Record resolved ranges for `x="y"` where `x` is an input or a constructor `@Attribute`. If its an input, check that strings are assignable to it. If not, give it its own error that hopefully explains the semi magical situation. * Track/typecheck std attrs Eventually we should * report errors for <a href="foo" [href]="bar"> * not suggest href after [href] has been used * not suggest [href] after href has been used * Changes based on feedback * Error codes added to hashmap so now show up. Fixed issue with <div template> causing crash * Changed pubspec to pull from master in angular_ast * Template parsing fix (#279) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Fix issue that caused empty binding on template attribute to crash * Now ready: Priority angular analysis (#273) * Priority dart & html analysis * Starter class, await supplementing errors or they're added too late. * Report has work to analyze for priority requests * Don't await for requesting files, do have a cache busting option * Handle case of files being requested multiple times * Priority html requests, requires serving errors with line infos. * Report all html files for all dart contexts in one method, plus tests I think before we were getting a last-one-wins effect, where the last analyzed dart/html pair would cover up all the errors for every other dart/html pair. As such, I need to carefully test that this change doesn't introduce new false errors (from them being covered up before). But now with this, we can report html errors back for multiple contexts from a single method when that's requested. Next I just have to use that method on getErrors when we have multiple relationships tracked already. * Guess html/dart relationship when none exist, otherwise use what's known using isEmpty rather than == 0 * Get setterType by setter.parameters[0] instead of setter.variable.type (#283) Seems like in the precense of a getter, setter.variable.type is the getter type, not the setter's type. So use the first function argument instead. * Remove contextRoot, won't be required in master soon, and breaks dev (#284) * Disable global attr plaintext typechecking until #280 has a clear solution (#286) * Update test_reflective_loader, fix analyzer warnings&errors (#285) * Update test_reflective_loader, fix analyzer warnings&errors * Fix the single server test too, which prevents build errors * Use var instead of types in angular_driver_test * Add TODOs * Percentage use fix in style.width and style.height. Test cases added (#282) * Percentage use fix in style.width and style.height. Test cases added * Added more property names that use percentage
mk13
added a commit
that referenced
this pull request
May 15, 2017
* Defend against files hidden by generated files (#271) * Travis sdk fix (#272) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Tick to 8 (#277) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Tick to 0.0.8 * Track @Attribute annotated constructor parameters (#270) * Resolve plain attributes -- in ranges, and strchecks for inputs. (#278) * Resolve plain attributes -- in ranges, and strchecks for inputs. Record resolved ranges for `x="y"` where `x` is an input or a constructor `@Attribute`. If its an input, check that strings are assignable to it. If not, give it its own error that hopefully explains the semi magical situation. * Track/typecheck std attrs Eventually we should * report errors for <a href="foo" [href]="bar"> * not suggest href after [href] has been used * not suggest [href] after href has been used * Template parsing fix (#279) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Fix issue that caused empty binding on template attribute to crash * Now ready: Priority angular analysis (#273) * Priority dart & html analysis * Starter class, await supplementing errors or they're added too late. * Report has work to analyze for priority requests * Don't await for requesting files, do have a cache busting option * Handle case of files being requested multiple times * Priority html requests, requires serving errors with line infos. * Report all html files for all dart contexts in one method, plus tests I think before we were getting a last-one-wins effect, where the last analyzed dart/html pair would cover up all the errors for every other dart/html pair. As such, I need to carefully test that this change doesn't introduce new false errors (from them being covered up before). But now with this, we can report html errors back for multiple contexts from a single method when that's requested. Next I just have to use that method on getErrors when we have multiple relationships tracked already. * Guess html/dart relationship when none exist, otherwise use what's known using isEmpty rather than == 0 * Get setterType by setter.parameters[0] instead of setter.variable.type (#283) Seems like in the precense of a getter, setter.variable.type is the getter type, not the setter's type. So use the first function argument instead. * Remove contextRoot, won't be required in master soon, and breaks dev (#284) * Disable global attr plaintext typechecking until #280 has a clear solution (#286) * Update test_reflective_loader, fix analyzer warnings&errors (#285) * Update test_reflective_loader, fix analyzer warnings&errors * Fix the single server test too, which prevents build errors * Use var instead of types in angular_driver_test * Add TODOs * Percentage use fix in style.width and style.height. Test cases added (#282) * Percentage use fix in style.width and style.height. Test cases added * Added more property names that use percentage * Update readme (#292) * Dont rehash html contents unless they change (#293) * update dependencies to point to the right place (#296) * Fix moved files (#298) * Fix moved files * Fix server test imports * Autocompletion fix (#297) * Checkpoint * html completion working, dart completion groundwork laid in, just started test * temp revert * Cleaned up completion functionality. Begin testing changes. * Small revert * Test cases fully passing * Removing unused imports * Remove linking of .dart completion - temporarily disabled * view.elementTags is now lazily computed. 'computeSuggestions' will always ensure that standardHtml is calculated before usage * getTemplateForFile now retuns a list of templates. computeSuggestions modified to adjust * Naming change for better accuracy; minor bug fix * @ContentChild and @ContentChildren validation. (#291) * @ContentChild and @ContentChildren validation. Validates: - ElementRef - TemplateRef - Components - Directives - String-based let bindings (and checks property match) - all others as errors - property has QueryList assignability for ContentChildren - ContentChild is only matched once Changes: - elements matching ContentChild of parent are counted as transcluded, error wise - DirectiveResolver is split into two phases, so that transclusions can be checked _after_ inner content is directive resolved (for the sake of ContentChild) - added "StandardAngular" where we get certain special ClassElements before running any other analysis, so that we can check if some random type x is a supertype of angular's ElementRef. - added ContentChildBindings so we can track the results of this process to affect autocompletion suggestions, navigation, refactoring Important edge cases: - proper generic type checking for Iterable<T> as QueryList<T> - using isSupertypeOf instead of isAssignableTo, because downcasting is _known_ to be wrong (otherwise in dart downcasting is assumed deliberate) - don't supress transclusion errors for element A within B just because some parent of B understands A as a contentChild -- has nothing to do with A not belonging to B. - don't report duplicate ContentChild errors when the duplicate is inside a prior match (in fact, don't even report assignability errors!) - directive matches require using exportAs with the #x="y" syntax. - getting the "constant value" fields with inheritance requires using `.getField("(super)")` the correct amount of times. This looks VERY fragile to me. - Native elements get Components, but they become ElementRefs not TableElements. Ensure people don't use @ContentChild(TableElement). Tests: - changed to use 'package:angular2' instead of '/angular2/' because that is required to build StandardAngular. However, not all tests are changed, because its still ok to use '/angular2' too. - added ElementRef and TemplateRef and QueryList classes. Todos: - ViewChild/ViewChildren - track matched #attrs instead of just Elements - revisit the highlighted ranges, can we make them more specific? - instantiate classes to bounds * Look for selector field with arbitrary inheritance * Optimize content child scanning * Fuzz test updates * Another fuzz fix * Avoid element.unit, very slow. Use enclosingElement * Fix server tests failing (#300) * Prevent inf loop of trying to resolve standardAngular in workspaces that don't have angular2 imported (#301) * Implement new methods that will soon be part of AnalysisDriverGeneric (#299) * Change setPriorityFiles to set priorityFiles (#304) * Change event emitter to stream (#305) Change event emitter to stream * Plain attribute autocompletion (#303) * Component and standard inputs - autocomplete as plain attribute * Autocomplete bug fix * Checkpoint * test cases added * Quick fix - still need to follow through null checks * Fix travisCL complaining, also remove unnecessary whitespace * Allows dynamic types to be autocompleted and improved flow * get typeprovider from template * Better validation of plain attributes (#306) * Fix pathing for byte_store (#312) * null check added on source (#311) * Remove searchEngine from CompletionRequest reflecting changes made in… (#314) * Remove searchEngine from CompletionRequest reflecting changes made in completion core, and remove index as per suggestion * Cleaned up unused imports and unused override reflecting recent changes (#315) * Cleaned up unused imports and unused override reflecting recent changes * remove context from completionRequest * Removing unnecessary null check * Omega lint fixup (#313) * Get new plugin architecture in a workable development stage (#316) See readme for how to get set up....currently doesn't do anything, though. * More lint fixes, handle moved file (#317) * Autocomplete stars (#310) * Complete inputs inside of template elements Complete, for instance, `trackBy` inside of an ngFor. * Analyze and suggest directives that appear to want `*star` syntax. If a `TemplateRef` is in the constructor, it works with star syntax. Warn when not used with star syntax, and warn when star syntax is used on a directive that does not have this special ctor as otherwise the template will go unused. Its not a foolproof system, I don't think. So detect that separately from detection of ngIf and ngFor without stars. Especially so that in the cases where this goes wrong, users can opt out of the error without losing errors for those core directives' validation...and give different messages. Not sure what's more likely...that this expects too many things to be used with stars, or not enough... Also suggest this in autocomplete for great profit. Assume that these directives rely on a property binding, so suggest *p for all p which are property selectors. Note this is also a less than perfect system, as it includes suggesting `*ngForOf`, since that's part of `NgFor`s selector. Hardcode that out, at least for now. With this plus suggesting `trackBy:`, we now (I think) suggest everything relevant for two way autocomplete except `template=` attributes themselves and the `let` keyword. Wanted to do the latter but it seemed like it might be a bit treacherous: think `ngIf="^"` vs `ngFor="^"`, it would be the first time we'd suggest both dart and non-dart things at once. * Fix analysis errors * prefer var and final over types * More lint fixes * Include colon in suggestion * Move performance log (#318) * Tests passing * Sync to master; tests passing
mk13
added a commit
that referenced
this pull request
May 16, 2017
* Summary integration (#276) * Sync to origin master * checkpoint * Initial ast integration - ranges not behaving properly * Minor bug fix * checkpoint * Tasks tests all passing * Checkpoint before error code fix * Tasks tests set back to 100% after introducing error codes * resolver_test midway checkpoint * Checkpoint * Full test coverage in analyzer. Server still left * All tests passing * Make paths explicit * Remove comments * Attributes now sorted by offset. Improves performance in auto completion * DOCTYPE now acceptable * Created separate Ast for top level document container * Refactored primary function in completion.dart * Minor code clean up * Fuzz tester bug fix * Revert try catch block * Rename exception codes after changes in angular_ast * Bug fix on [class. [style. [attr. bindings; range error * Allow spaces in msutaches * Autocompletion in text attribute value is now empty * Allow for plain [class] binding * Fixed update_deps.sh to work with angular_ast * Clean up deps and remove some unnecessary comments * Changed pubspec to pull from master * Defend against files hidden by generated files (#271) * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Travis sdk fix (#272) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * First round of quick fixes based on feedback * remove comment * Error codes no longer use hard offsets; uses string instead * Merged attribute generating logic to be agnostic of Element or template type. * Assert for indexOf added into test. Moved comments for clarity * Remove comment-blocked chunk * Changes based on feedback * Error codes added to hashmap so now show up. Fixed issue with <div template> causing crash * Changed pubspec to pull from master in angular_ast * Summary integration sync (#288) * Sync to origin master * checkpoint * Initial ast integration - ranges not behaving properly * Minor bug fix * checkpoint * Tasks tests all passing * Checkpoint before error code fix * Tasks tests set back to 100% after introducing error codes * resolver_test midway checkpoint * Checkpoint * Full test coverage in analyzer. Server still left * All tests passing * Make paths explicit * Remove comments * Attributes now sorted by offset. Improves performance in auto completion * DOCTYPE now acceptable * Created separate Ast for top level document container * Refactored primary function in completion.dart * Minor code clean up * Fuzz tester bug fix * Revert try catch block * Rename exception codes after changes in angular_ast * Bug fix on [class. [style. [attr. bindings; range error * Allow spaces in msutaches * Autocompletion in text attribute value is now empty * Allow for plain [class] binding * Fixed update_deps.sh to work with angular_ast * Clean up deps and remove some unnecessary comments * Changed pubspec to pull from master * Defend against files hidden by generated files (#271) * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Travis sdk fix (#272) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * First round of quick fixes based on feedback * Tick to 8 (#277) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Tick to 0.0.8 * remove comment * Error codes no longer use hard offsets; uses string instead * Track @Attribute annotated constructor parameters (#270) * Merged attribute generating logic to be agnostic of Element or template type. * Assert for indexOf added into test. Moved comments for clarity * Remove comment-blocked chunk * Resolve plain attributes -- in ranges, and strchecks for inputs. (#278) * Resolve plain attributes -- in ranges, and strchecks for inputs. Record resolved ranges for `x="y"` where `x` is an input or a constructor `@Attribute`. If its an input, check that strings are assignable to it. If not, give it its own error that hopefully explains the semi magical situation. * Track/typecheck std attrs Eventually we should * report errors for <a href="foo" [href]="bar"> * not suggest href after [href] has been used * not suggest [href] after href has been used * Changes based on feedback * Error codes added to hashmap so now show up. Fixed issue with <div template> causing crash * Changed pubspec to pull from master in angular_ast * Template parsing fix (#279) * Sync to origin master * Fix travis build. Fix sdk related errors (ContextRoot and IdeOptions). Clean up imports to remove unused * Fix issue that caused empty binding on template attribute to crash * Now ready: Priority angular analysis (#273) * Priority dart & html analysis * Starter class, await supplementing errors or they're added too late. * Report has work to analyze for priority requests * Don't await for requesting files, do have a cache busting option * Handle case of files being requested multiple times * Priority html requests, requires serving errors with line infos. * Report all html files for all dart contexts in one method, plus tests I think before we were getting a last-one-wins effect, where the last analyzed dart/html pair would cover up all the errors for every other dart/html pair. As such, I need to carefully test that this change doesn't introduce new false errors (from them being covered up before). But now with this, we can report html errors back for multiple contexts from a single method when that's requested. Next I just have to use that method on getErrors when we have multiple relationships tracked already. * Guess html/dart relationship when none exist, otherwise use what's known using isEmpty rather than == 0 * Get setterType by setter.parameters[0] instead of setter.variable.type (#283) Seems like in the precense of a getter, setter.variable.type is the getter type, not the setter's type. So use the first function argument instead. * Remove contextRoot, won't be required in master soon, and breaks dev (#284) * Disable global attr plaintext typechecking until #280 has a clear solution (#286) * Update test_reflective_loader, fix analyzer warnings&errors (#285) * Update test_reflective_loader, fix analyzer warnings&errors * Fix the single server test too, which prevents build errors * Use var instead of types in angular_driver_test * Add TODOs * Percentage use fix in style.width and style.height. Test cases added (#282) * Percentage use fix in style.width and style.height. Test cases added * Added more property names that use percentage * Tests passing * Sync to master; tests passing
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on: https://codereview.chromium.org/2815643004/
Turns out this is also required for vim.
Still have to improve requested html analysis, as it currently just assumes
x.dartandx.htmlrelate. This is because otherwise we'd have to wait until allllll dart analyses finish before we can start the priority html analysis. However, sometimes all dart is already analyzed. In this case, we should go off of what we know are the latest links between files.