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

Remove FunctionScope and improve type tracking #258

Closed
wants to merge 25 commits into from

Conversation

TwitchBronBron
Copy link
Member

The variable defined in for each statements were not showing up for completions. This PR fixes that.

src/files/BrsFile.ts Outdated Show resolved Hide resolved
@TwitchBronBron
Copy link
Member Author

So the scope of this PR kinda spiraled out of control a bit, but I think for the better. I did still fix the for-each-var completions issue. However, I also made a few other changes:

  • completely removed the FunctionScope concept, in favor of capturing variable declarations for each function at parse-time and storing them in references. This cut down on a lot of after-parse looping.
  • Add LazyType to defer deriving the type of certain variable types until after the file has been parsed.

This change yielded some fairly impressive performance improvements for the validate cycle:
image

@elsassph I'm ready for your review again.

src/parser/Parser.ts Outdated Show resolved Hide resolved
src/parser/Parser.ts Outdated Show resolved Hide resolved
src/parser/Parser.ts Outdated Show resolved Hide resolved
src/parser/Parser.ts Outdated Show resolved Hide resolved
@TwitchBronBron TwitchBronBron changed the title For each var completions Remove FunctionScope and improve type tracking Jan 22, 2021
@TwitchBronBron TwitchBronBron force-pushed the for-each-var-completions branch from a30d89c to f2dc8f0 Compare January 30, 2021 18:36
@chrisdp
Copy link
Contributor

chrisdp commented Mar 16, 2021

Is this ready for reviewing? If so I might be interested in hopping on a call for this one due to the scope and size.

@TwitchBronBron
Copy link
Member Author

No, this isn't ready for reviewing yet. I still need to refactor some of typing stuff to look into scopes rather than just per-file.

TwitchBronBron and others added 5 commits March 16, 2021 05:59
* Ran npm audit and fixed vulvnerability

* Added symbol table class

* Parser adds to a symbol table as it parses code

* Revert package changes

* Fixed test that was referencing LazyType when it was not supposed to

* Better handling of a variable potentially changing type. Added getSymbolType() function and fixed error of a function inheriting symbols from a containing function

* Fixed typo

* Removed code comment

* Fixes from PR comments - and made sure for each loop variables were added to the symbolTable

* Changed BscSymbol interface to include a range

Co-authored-by: Mark Pearce <[email protected]>
* Adds SymbolTable.hasKey method

* Makes CustomType display its actual type

* Standardize how a FunctionType is created from an expression

* Adds symbol tables to namespaces

* Adds a symbol table to scope, and computes merged symbol tables across files

* Use the symbolTable on getHover

* Working on argument type mismatch validation

* Proper type tracking across files in the same scope, including namespaces. Adds validation on argument types

* Fixed bad whitespace in tests

* Updates from code review

* Fixed all circular dependency issues

* Hopefully fixed all whitespace issues

* Fixes for when scope changes

* Fixed lint error and remove symbolTable clear - it was not needed

* Clean up getHover code

Co-authored-by: Mark Pearce <[email protected]>
@TwitchBronBron
Copy link
Member Author

Closing in favor of the v1 PR #401

@TwitchBronBron TwitchBronBron deleted the for-each-var-completions branch June 8, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants