Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Drop AST for libraries after final analysis #1134

Merged
merged 50 commits into from
Jun 7, 2019

Conversation

MikhailArkhipov
Copy link

@MikhailArkhipov MikhailArkhipov commented May 28, 2019

Fixes #1088

Includes #1133

  • Remove AST nodes from types
  • Cache AST nodes at module level so they cache can be cleared when analysis is done.
  • Occasionally analysis is requested again even when previous was claimed to be final, so add code to reload and reparse the file. This may be related to Stubs aren't analyzed before their modules #907.
  • Keep AST for stubs generated by the scraper since some functions there require evaluation with arguments
  • Remove reference to the AST root from nodes, reducing node size.
  • Add qualified name to types in preparation for their identification in persistent models

@MikhailArkhipov MikhailArkhipov changed the title WIP: drop AST for libraries after final analysis Drop AST for libraries after final analysis May 30, 2019
@jakebailey
Copy link
Member

One thing to check, CompletionTest.InImport is consistently failing for me.

 InImport
   Source: CompletionTests.cs line: 745
   Duration: 11 sec

  Message: 
    Expected completion list items to have label 'TestCase', but it has none.
  Stack Trace: 
    at LateBoundTestFramework.Throw(String message) in LateBoundTestFramework.cs line: 16
    at AssertionScope.FailWith(String message, Object[] args) in AssertionScope.cs line: 225
    at CompletionResultAssertions.HaveAttribute(IEnumerable`1 attributes, Func`2 attributeSelector, String itemNameSingle, String itemNamePlural, String because, Object[] reasonArgs) in CompletionResultAssertions.cs line: 113
    at CompletionResultAssertions.HaveLabels(IEnumerable`1 labels, String because, Object[] reasonArgs) in CompletionResultAssertions.cs line: 95
    at CompletionResultAssertions.HaveLabels(String[] labels) in CompletionResultAssertions.cs line: 91
    at CompletionTests.InImport() in CompletionTests.cs line: 789

   Open additional output for this result

@MikhailArkhipov
Copy link
Author

Passes on 2017 and 2019, although there is some flakiness in tests that involve imports, I had 3 tests (different) failed but passed on rerun (re compile, datetime delta and some other). This is actually b/c 'analysis ready' event on the module is set when one of the sessions completes. Hence if module is analyzed before stub, as we have in the bug, analysis may end up incorrect as await returns with the analysis without the stub and not with the final one, which happens later. This happens in master as well.

@MikhailArkhipov MikhailArkhipov merged commit b35bfe8 into microsoft:master Jun 7, 2019
@MikhailArkhipov MikhailArkhipov deleted the noast5 branch June 7, 2019 14:24
@MikhailArkhipov MikhailArkhipov mentioned this pull request Jun 7, 2019
jakebailey pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
* Remove old qualified name

* Node storage

* Class and scope to use AST map

* Library analysis

* Fix SO

* Keep small AST with imports

* AST reduction

* Final field

* Reload

* Ignore post-final requests

* Drop AST

* Remove local variables

* Test fixes

* Fix overload match

* Tests

* Add locks

* Remove local variables

* Drop file content to save memory

* Cache PEP hints

* Recreate AST

* Fix specialization

* Fix locations

* usings

* Test fixes

* Add options to keep data in memory

* Fix test

* Fix lambda parameters

* Fix argument set
Fix global scope node

* Fix overload doc

* Fix stub merge errors

* Fix async issues

* Undo some changes

* Fix test

* Fix race condition

* Restore log null checks

* Fix merge conflict

* Fix merge issue

* Null check

* Fix test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove references to AST from analysis
2 participants