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

Type Tracking #783

Merged
merged 28 commits into from
Jun 6, 2023
Merged

Type Tracking #783

merged 28 commits into from
Jun 6, 2023

Conversation

TwitchBronBron
Copy link
Member

@TwitchBronBron TwitchBronBron commented Mar 16, 2023

This is the first of several type tracking tickets.

Tasks:

  • 1. Add a new AstNode class called TypeExpression.

    It will be the base AstNode for all future type expressions. The following existing code will be parsed in this way:

    the as type in function paramenters

    'the `string`, `integer`, and `SomeInterface` parts should be TypeExpression. The `as` belongs to the 
    `FunctionParameterExpression`
    sub doSomething(p1 as string, p2 as integer, p3 as SomeInterface)
    end sub

    as ReturnType for function return types

    'the `boolean` should be a TypeExpression. the `as` belongs to the FunctionExpression
    function doSomething() as boolean

    as MemberType for class and interface types

    class Video
        ' the `boolean` should be a TypeExpression, the `as` belongs to the FieldStatement
        public isPlaying as boolean
    end class
    
    interface Metadata
        ' the `string` should be a TypeExpression, the `as` belongs to the FieldStatement
        url as string
    end interface

    This is a breaking change to the AST. For example, FunctionParameterExpression will no longer have a typeToken, but will instead have a typeExpression.

    Here's a rough idea of the structure of the TypeExpression class

    class TypeExpression extends Expression
        constructor(
            public expression:Expression
        ){}
    
        public getType(){
            if (isVariableExpression(this.expression) ){
                return this.getSymbolTable().getType(this.expression.name.text);
            } else if (isDottedGetExpression(this.expression)) { 
                //look up the leftmost variable (i.e. `ns1.ns2.ns3.Iface` would get us `ns1`
                //then, use the BscType interfaces to dig down into the type to get the type
            } else {
                //maybe other stuff?
            }
        }
  • 2. Update the parser to properly create subclasses that extend TypeExpression for (no longer necessary)

    Since all AstNode instances are equipped with a .getSymbolTable() function and BrighterScript already links the symbol tables to the correct node level, the TypeExpression nodes will not need any type of linking phase. These will be automatically resolved and avaiable to the SymbolTable

  • 3. Add a getType() method to the base AstNode, which can then be overridden by all AstNode descendents.

    Moving forward, every AstNode will have the potential to have a type. To improve stability of the project, the nodes should return "DynamicType" when the type is not known.

    abstract class AstNode
        public abstract getType(): BscType {
            //TODO implement for every node. 
       }
        //...all the other props
    end class
  • 4. Enhance the SymbolTable BscType logic
    The current SymbolTable implementation only adds names, but no types (they're all added as dynamic). This needs to be enhanced to actually add symbol types.

    Consider the following code example.

    function main()
        name1 = "John" 
        name2 = name1 '
        name3 = name2 '
    end function

    The symbol table logic might look something like this:

    const table = new SymbolTable();
    table.AddSymbol("name1", new StringType());
    table.addSymbol("name2", new ReferenceType("name1"));
    table.addSymbol("name3", new ReferenceType("name2"));

    This would happen at the start of onProgramValidate for every file. Then, for every scope, the scope symbol tables would be linked to the file, and these Types would be able to derive their information from the parent symbol table accordingly.

    We should probably cache the type lookups during every scope validation, and clear it at the end of each scope validation. Otherwise we'd spend many cycles reevaluating the same types.

  • 5. Validate all TypeExpression AstNodes to ensure they reference valid types that can be found in the symbol table

  • 6. Modify SymbolTable to always return a single type. That means if there are multiple different symbol types represented, they should be merged into a UnionType.

  • 6b. Add way to actually declare a Union Type in code:

function intValue(num as string or integer) as integer
   if type(num) = "Integer"
     return num
   end if
   return Val(num, 10)
end function 
  • 6c. Make UnionType.getMemberTypes look at the intersection of member types of all the UnionTypes' types. And since this will require coding, I will probably also need to type it (with a keyboard!)

  • 7. Type chains - For DottedGets, IndexedGets, etc. how do we let the developer know where the chain failed its understanding the current type.

  • 8. Remove NamespacedVariableExpression in favour of TypeExpression except NamespaceStatement.name which should be DottedGetExpression

  • 9. Add type caching, at the appropriate (scope?) level. We must make sure that non-resolved types don't get cached

  • 10. Simplify SymbolTable.getSymbolType to use type (if available), first from cache, then from table, otherwise create ReferenceType, if TableProvider is given, and cache result.

Future Work (Non-Breaking, Eg. Phase 2 or 3)

  • Allow TypeExpressions in more locations (eg. AssignmentStatements, etc)

  • BinaryExpressions and UnaryExpressions should be able to infer resultant types (eg. Integer * Integer => Integer, String + String => String, etc.)

  • Change Hover & Completion to use SymbolTable instead of variableDeclarations/callables
    This will complete the refactor to move away from using these lookup tables to unify how a symbol can be discovered/known. When this is done, the concept of VariableDeclarations can be removed from the code.

  • Validate various expressions for type validity

    AssignmentStatement type equality

    sub main()
        name = "bob"
        name = 1
    end sub
    '      ~ type 'Integer' cannot be assigned to type 'String'

    Function argument equality

    sub main()
        printMessage(1)
        '            ~ Argument of type 'number' is not assignable to parameter of type 'string'
    end sub
    function printMessage(message as string)
        print message
    end function
  • Add ComponentType that are generated from XML or future Component creation. Members (and member types) could be inferred from the component's fields

  • CreateObject() should infer correct type (using data scraped from Roku SceneGraph docs for built in types, or from known components)

  • Update ArrayType to allow indexedGet, etc. to infer type. Allow code & validation like:

  function sumArray(numbers as integer[]) as integer
       sum = 0
       for each num in numbers
           sum = sum + num ' num is known to be an integer
       end for
       return sum
   end function
   
   sub addName(nameList as string[])
        nameList.push(3.14) ' validation error --- nameList.push expects string
    end sub

   ```

@TwitchBronBron TwitchBronBron changed the title Add abstract TypeExpression class Type Tracking Mar 17, 2023
TwitchBronBron and others added 24 commits April 13, 2023 09:38
* Convert type tokens to TypeExpressions

* Fixed all tests

* Rebased and fixed any other merge issues

* Further work on SymbolTypeFlags

* Lint fixes

* Fixed issue with using enums as variables

* Uses complete TypeName as Custom type for TypeExpression.getType()

* Removed commented out code, fixed tests

* Fixes from PR review

* Fixed lint issues

---------

Co-authored-by: Mark Pearce <mark.pearce@redspace.com>
* Refactors statically set type to use a function

* Lint fixes

* Removed symbolTable getter from function scope - not used
* Added ReferenceType and some tests

* Added scope linking for hovers

* Removed catch in try/finally in hoverprocessor

* Reverted typo

* Removed commented out code

* Changed to using Proxy, fixed circular references

* Fixed lint, added proper isReferencetype reflection
…794)

* Large update to move BscType to a class and add ReferenceTypes

* Fixes validation errors when calling a dynamicType like a function

* Correctly resolves return types after a function call.

* Removed some commented out or dead code

* Fixed build errors

* BscType.symbolTable -> memberTable, PropertyReferenceType->TypePropertyReferenceType

* Fixed test that should fail
* Fixed issues with namespaces and invalid things being used as types

* Removed unneeded method

* Ran npm install with latest code

* Removed commented and vestigal code, and added TODOs

* Removed additional commented code
* Changed CustomType to ClassType

* Adds BscType.equals(), and ClassType tests for inheritance

* Fixes issue when looking for members of classes with supertypes that were originally ReferenceTypes

* Rerfactor of InterfaceStatement and InetrfaceType work more similar to Classes

* Added tests, changed parentClass/Interface to use TypeExpression

* Fixes issue when classes and namespaces extend from a namespaced parent

* Changes from code review
* Changed getType() methods to use an options parameter

* Refactors getType() to add opttions, and can find all the types in a chain of types

* Fixed lint errors

* Updated Tests

* Update src/interfaces.ts

Co-authored-by: Bronley Plumb <bronley@gmail.com>

* name change

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>
* initial commit

* Adds UnionType and associated tests

* Added helper functions to get the most general type from a list of types, including Union types

* Started working on Uniontype.membersTypes

* Started working on Uniontype.membersTypes

* Refactor of types to use  instead of

* Fixed lint errors
* Removes NamespacedVariabledNamedExpressions

* Fixed rest of tests

* Fixes ignored parameter value in Parser.identifyingExpression
* Reworked type uniqness checking

* Changed logic of InterfaceType.isEqual

* Able to get member types from a UnionType

* Fixed lint errors

* Starting to add UnionExpression and parsing

* Added pipe token

* Added test for if inner type does not include member in UnionType.getMmeberTypes

* Updated UnionExpression transpile logic

* Re-added whitespace

* removed pipe in favor of 'or' for declaring/displaying union types

* Fix lint/build issues

* TypeExpressions more narrowly look for what they can accept

* Whitespace and typo fixes

* Adds unionType validation tests and allows uniontypes innertypes to be late defined

* fixed lint error

* Fixes issue with ReferenceTypes not displaying a full name
* Fixes issue with self-referencing symbols in unions

* Fixed lint error
* Fixes issue with self-referencing symbols in unions

* Fixed lint error

* Starting performance benchmarking

* Starting performance benchmarking

* getType -> ZgetType()

* Reverse getType removal

* Removes double validation in ScopeValidator

* Trying out type caching

* Some benchmark thing

* Removes TypeCache

* Made the in typeExpression function more robust

* Removed unneeded code

* removed bad imports

* Fixes issue with potential crash when checking types
* Added bsConfig option for enhancedTypingValidation

* Reverted any benchmark changes

* Added docs for 'enhancedTypingValidation' option

* Adds documentation

* In an effort to increase non-enhancedtypeValidation, old methods were re-added to short circuit getType() calls

* Changed flag to enableTypeValidation

* Fixed docs

* Fixed readme formatting

* Fixes from PR review
…eation (#813)

* Adds SymbolTable.getSymbolType() to wrap caching and ReferenceType creation

* Removed commented out code

* Add SymbolTable exports for plugins, etc

* Added exports from ./types
@TwitchBronBron TwitchBronBron changed the base branch from master to release-0.66.0 June 5, 2023 11:57
TwitchBronBron and others added 2 commits June 5, 2023 07:57
* Adds TypeCastExpression

* Adds TypeCast expression

* fix lint issue

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>
@TwitchBronBron TwitchBronBron merged commit f144743 into release-0.66.0 Jun 6, 2023
5 checks passed
@TwitchBronBron TwitchBronBron deleted the typing-phase1 branch June 6, 2023 17:46
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.

None yet

2 participants