diff --git a/README.md b/README.md index 8f4c60a..442f8fd 100644 --- a/README.md +++ b/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`. diff --git a/src/index.ts b/src/index.ts index da78db6..44198d0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,6 +52,7 @@ export type BsLintConfig = Pick ({ + message: `${ST} Reassignment of the type of '${varName}' from ${previousType} to ${newType}`, + code: CodeStyleError.NameShadowing, + severity: severity, + source: 'bslint' }) }; diff --git a/src/plugins/codeStyle/index.spec.ts b/src/plugins/codeStyle/index.spec.ts index 6fc9542..d2bee23 100644 --- a/src/plugins/codeStyle/index.spec.ts +++ b/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 = [ diff --git a/src/plugins/codeStyle/index.ts b/src/plugins/codeStyle/index.ts index d9a4a16..b9c3113 100644 --- a/src/plugins/codeStyle/index.ts +++ b/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(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 + }); + } + } + } + } } /** diff --git a/src/util.ts b/src/util.ts index 4969825..9ebd0e8 100644 --- a/src/util.ts +++ b/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']) }; } diff --git a/test/project1/source/type-reassignment-custom.bs b/test/project1/source/type-reassignment-custom.bs new file mode 100644 index 0000000..85486e5 --- /dev/null +++ b/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 diff --git a/test/project1/source/type-reassignment.brs b/test/project1/source/type-reassignment.brs new file mode 100644 index 0000000..27097e1 --- /dev/null +++ b/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