Adds diagnostic for when a variables type changes (#130)

* Adds diagnostic for when a variables type changes

* Some more tests around object type

README.md

@@ -269,6 +269,8 @@ Default rules:
 
 - `name-shadowing`: enforces that no two items have the same name (`error | warn | info | off`)
 
+- `type-reassignment`: enforces that a variable is not used to reference multiple types (`error | warn | info | off`)
+
 ### Code flow rules
 
 Valid values for the rules severity are: `error | warn | info | off`.

src/index.ts

@@ -52,6 +52,7 @@ export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd
         'no-assocarray-component-field-type'?: RuleSeverity;
         'no-array-component-field-type'?: RuleSeverity;
         'name-shadowing'?: RuleSeverity;
+        'type-reassignment'?: RuleSeverity;
     };
     globals?: string[];
     ignores?: string[];
@@ -88,6 +89,7 @@ export interface BsLintRules {
     noAssocarrayComponentFieldType: BsLintSeverity;
     noArrayComponentFieldType: BsLintSeverity;
     nameShadowing: BsLintSeverity;
+    typeReassignment: BsLintSeverity;
 }
 
 export { Linter };

src/plugins/codeStyle/diagnosticMessages.ts

@@ -222,5 +222,11 @@ export const messages = {
         code: CodeStyleError.NameShadowing,
         severity: severity,
         source: 'bslint'
+    }),
+    typeReassignment: (varName: string, previousType: string, newType: string, severity: DiagnosticSeverity) => ({
+        message: `${ST} Reassignment of the type of '${varName}' from ${previousType} to ${newType}`,
+        code: CodeStyleError.NameShadowing,
+        severity: severity,
+        source: 'bslint'
     })
 };

src/plugins/codeStyle/index.spec.ts

@@ -1058,6 +1058,38 @@ describe('codeStyle', () => {
         });
     });
 
+    describe('type-reassignment', () => {
+        it('detects type reassignment', async () => {
+            const diagnostics = await linter.run({
+                ...project1,
+                files: ['source/type-reassignment.brs'],
+                rules: {
+                    'type-reassignment': 'error'
+                }
+            });
+            expectDiagnosticsFmt(diagnostics, [
+                '12:LINT3026:Strictness: Reassignment of the type of \'param\' from string to integer',
+                '18:LINT3026:Strictness: Reassignment of the type of \'value\' from integer to string',
+                '27:LINT3026:Strictness: Reassignment of the type of \'value\' from integer to dynamic',
+                '53:LINT3026:Strictness: Reassignment of the type of \'obj\' from integer to roAssociativeArray'
+            ]);
+        });
+
+        it('allows type reassignment with custom types', async () => {
+            const diagnostics = await linter.run({
+                ...project1,
+                files: ['source/type-reassignment-custom.bs'],
+                rules: {
+                    'type-reassignment': 'error'
+                }
+            });
+            expectDiagnosticsFmt(diagnostics, [
+                '30:LINT3026:Strictness: Reassignment of the type of \'arg\' from Iface1 to roAssociativeArray',
+                '44:LINT3026:Strictness: Reassignment of the type of \'arg\' from Child to Parent'
+            ]);
+        });
+    });
+
     describe('fix', () => {
         // Filenames (without the extension) that we want to copy with a "-temp" suffix
         const tmpFileNames = [

src/plugins/codeStyle/index.ts

@@ -27,7 +27,10 @@ import {
     ExtraSymbolData,
     OnScopeValidateEvent,
     InternalWalkMode,
-    isCallableType
+    isCallableType,
+    AssignmentStatement,
+    isFunctionExpression,
+    isDynamicType
 } from 'brighterscript';
 import { RuleAAComma } from '../..';
 import { addFixesToEvent } from '../../textEdit';
@@ -231,7 +234,7 @@ export default class CodeStyle implements CompilerPlugin {
     validateBrsFileInScope(file: BrsFile) {
         const diagnostics: (BsDiagnostic)[] = [];
         const { severity } = this.lintContext;
-        const { nameShadowing } = severity;
+        const { nameShadowing, typeReassignment } = severity;
 
         file.ast.walk(createVisitor({
             NamespaceStatement: (nsStmt) => {
@@ -248,6 +251,9 @@ export default class CodeStyle implements CompilerPlugin {
             },
             EnumStatement: (enumStmt) => {
                 this.validateNameShadowing(file, enumStmt, enumStmt.tokens.name, nameShadowing, diagnostics);
+            },
+            AssignmentStatement: (assignStmt) => {
+                this.validateTypeReassignment(file, assignStmt, typeReassignment, diagnostics);
             }
             // eslint-disable-next-line no-bitwise
         }), { walkMode: WalkMode.visitStatementsRecursive | InternalWalkMode.visitFalseConditionalCompilationBlocks });
@@ -459,6 +465,44 @@ export default class CodeStyle implements CompilerPlugin {
             relatedInformation: relatedInformation
         });
     }
+
+    validateTypeReassignment(file: BrsFile, assignStmt: AssignmentStatement, severity: DiagnosticSeverity, diagnostics: (BsDiagnostic)[]) {
+        const functionExpression = assignStmt.findAncestor<FunctionExpression>(isFunctionExpression);
+        if (!functionExpression) {
+            return;
+        }
+        const bodyTable = functionExpression.body?.getSymbolTable();
+        const rhsType = assignStmt.value?.getType({ flags: SymbolTypeFlag.runtime });
+        if (!rhsType.isResolvable()) {
+            return;
+        }
+        const varName = assignStmt.tokens.name.text;
+        let previousType = functionExpression.getSymbolTable().getSymbolType(varName, { flags: SymbolTypeFlag.runtime });
+
+        if (!previousType) {
+            // check for last previous assignment
+            const symbols = bodyTable.getSymbol(varName, SymbolTypeFlag.runtime);
+            for (const symbol of symbols) {
+                if (util.comparePosition(symbol.data?.definingNode?.location?.range?.start, assignStmt.location.range.start) < 0) {
+                    previousType = symbol.type;
+                } else {
+                    break;
+                }
+            }
+        }
+
+        if (previousType?.isResolvable()) {
+            // is this different?
+            if (!isDynamicType(previousType)) {
+                if (isDynamicType(rhsType) || !previousType.isTypeCompatible(rhsType)) {
+                    diagnostics.push({
+                        ...messages.typeReassignment(varName, previousType.toString(), rhsType.toString(), severity),
+                        location: assignStmt.location
+                    });
+                }
+            }
+        }
+    }
 }
 
 /**

src/util.ts

@@ -29,7 +29,8 @@ export function getDefaultRules(): BsLintConfig['rules'] {
         'no-print': 'off',
         'no-assocarray-component-field-type': 'off',
         'no-array-component-field-type': 'off',
-        'name-shadowing': 'off'
+        'name-shadowing': 'off',
+        'type-reassignment': 'off'
     };
 }
 
@@ -168,7 +169,8 @@ function rulesToSeverity(rules: BsLintConfig['rules']) {
         colorCertCompliant: rules['color-cert'],
         noAssocarrayComponentFieldType: ruleToSeverity(rules['no-assocarray-component-field-type']),
         noArrayComponentFieldType: ruleToSeverity(rules['no-array-component-field-type']),
-        nameShadowing: ruleToSeverity(rules['name-shadowing'])
+        nameShadowing: ruleToSeverity(rules['name-shadowing']),
+        typeReassignment: ruleToSeverity(rules['type-reassignment'])
     };
 }
 

test/project1/source/type-reassignment-custom.bs

@@ -0,0 +1,51 @@
+interface Iface1
+    a as string
+end interface
+
+interface Iface2
+    a as string
+    optional b as string
+end interface
+
+interface OtherFace1
+    a as integer
+end interface
+
+function convertToIface1(arg as Iface2) as Iface1
+    arg = {a: arg.a} ' no diagnostic
+    return arg
+end function
+
+function convertToIface2(arg as Iface1) as Iface2
+    arg = {a: arg.a, b: "hello"} ' no diagnostic
+    return arg
+end function
+
+function convertToIface2ViaTypeCast(arg as Iface1)
+    arg = arg as Iface2 ' no diagnostic
+    return arg
+end function
+
+function convertToToOtherFace1(arg as Iface1)
+    arg as OtherFace1 = {a: 1} ' diagnostic
+    return arg
+end function
+
+
+class Parent
+    a as string
+end class
+
+class Child extends Parent
+    b as string
+end class
+
+function convertToParent(arg as Child)
+    arg = new Parent() ' diagnostic
+    return arg
+end function
+
+function convertToChild(arg as Parent)
+    arg = new Child() ' no diagnostic - Child is a subclass of Parent
+    return arg
+end function

test/project1/source/type-reassignment.brs

@@ -0,0 +1,54 @@
+sub checkTypesNoProblem(param as string) ' no diagnostic
+    param = "Hello " + param
+    num = 1234
+    num = 6.7
+    bool = true
+    if num > 3
+        bool = false
+    end if
+end sub
+
+sub checkTypesChangeFromParam(param as string)
+    param = 1 ' was string. now integer
+end sub
+
+
+sub checkTypesInFunc()
+    value = 1
+    value = "hello" ' was string. now integer
+end sub
+
+function getDynamic()
+    return 1
+end function
+
+sub checkTypesDefinedToDynamic()
+    value = 1
+    value = getDynamic() ' was integer. now dynamic
+end sub
+
+sub checkTypesNumberChange() ' no diagnostic
+    param = 1
+    param = 3.14
+end sub
+
+
+sub checkTypesDynamicToDefined() ' no diagnostic
+    value = getDynamic()
+    value = 1 ' was dynamic. now integer
+end sub
+
+sub checkTypesObject(obj as object)
+    if obj = invalid
+        obj = createObject("roAssociativeArray")
+    end if
+    obj.foo = "bar"
+end sub
+
+sub checkTypesObjectToPrimitive(obj as object)
+    obj = 3 ' This is allowed because primitive types can be boxed as objects
+end sub
+
+sub checkTypesPrimitiveToObject(obj as integer)
+    obj = createObject("roAssociativeArray") ' was integer. now associativearray
+end sub