add rule for inefficient interface types (#115)

* add rule for checking for inefficient interface types

* move interface type check to code style plugin and divide it into two rules

* add check for type attr existence

* add rules to readme, adjust messages

* adjust messages

* fix tests

* edit messages, fix readme

* fix linting errors, adjust range

README.md

@@ -161,7 +161,10 @@ Default rules:
         "unreachable-code": "info",
         "case-sensitivity": "warn",
         "unused-variable": "warn",
-        "consistent-return": "error"
+        "consistent-return": "error",
+
+        "no-assocarray-component-field-type": "off",
+        "no-array-component-field-type": "off"
     }
 }
 ```
@@ -248,6 +251,10 @@ Default rules:
     - `always`: ensures all white and black color-format values either match or are darker/ lighter than the minimum recommended values. For white the maximum value is `#EBEBEB` and for black the minimum value is `#161616`
     - `off`: do not validate (**default**)
 
+-   `no-assocarray-component-field-type`: Using 'assocarray' type in component markup can result in inefficient copying of data during transfer to the render thread. Use 'node' type if possible for more efficient transfer of data from the task thread to the render thread (`error | warn | info | off`)
+
+-   `no-array-component-field-type`: Using 'array' type in component markup can result in inefficient copying of data during transfer to the render thread. Use 'node' type if possible for more efficient transfer of data from the task thread to the render thread (`error | warn | info | off`)
+
 ### Strictness rules
 
 - `type-annotations`: validation of presence of `as` type annotations, for function
@@ -325,4 +332,4 @@ Running `bslint` with `--checkUsage` parameter will attempt to identify unused c
 - Any script/component not walked will produce a warning.
 
 ## Changelog
-Click [here](CHANGELOG.md) to view the changelog
+Click [here](CHANGELOG.md) to view the changelog

src/index.ts

@@ -49,6 +49,8 @@ export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd
         'color-alpha'?: RuleColorAlpha;
         'color-alpha-defaults'?: RuleColorAlphaDefaults;
         'color-cert'?: RuleColorCertCompliant;
+        'no-assocarray-component-field-type'?: RuleSeverity;
+        'no-array-component-field-type'?: RuleSeverity;
     };
     globals?: string[];
     ignores?: string[];
@@ -82,6 +84,8 @@ export interface BsLintRules {
     colorAlpha: RuleColorAlpha;
     colorAlphaDefaults: RuleColorAlphaDefaults;
     colorCertCompliant: RuleColorCertCompliant;
+    noAssocarrayComponentFieldType: BsLintSeverity;
+    noArrayComponentFieldType: BsLintSeverity;
 }
 
 export { Linter };

src/plugins/codeStyle/diagnosticMessages.ts

@@ -23,7 +23,9 @@ export enum CodeStyleError {
     ColorCase = 'LINT3020',
     ColorAlpha = 'LINT3021',
     ColorAlphaDefaults = 'LINT3022',
-    ColorCertCompliant = 'LINT3023'
+    ColorCertCompliant = 'LINT3023',
+    NoAssocarrayFieldType = 'LINT3024',
+    NoArrayFieldType = 'LINT3025'
 }
 
 const CS = 'Code style:';
@@ -199,5 +201,19 @@ export const messages = {
         source: 'bslint',
         message: `${CS} File should follow Roku broadcast safe color cert requirement`,
         range
+    }),
+    noAssocarrayFieldType: (range: Range, severity: DiagnosticSeverity) => ({
+        message: `Avoid using field type 'assocarray'`,
+        code: CodeStyleError.NoAssocarrayFieldType,
+        severity: severity,
+        source: 'bslint',
+        range
+    }),
+    noArrayFieldType: (range: Range, severity: DiagnosticSeverity) => ({
+        message: `Avoid using field type 'array'`,
+        code: CodeStyleError.NoArrayFieldType,
+        severity: severity,
+        source: 'bslint',
+        range
     })
 };

src/plugins/codeStyle/index.spec.ts

@@ -412,6 +412,7 @@ describe('codeStyle', () => {
         expect(actual).deep.equal(expected);
     });
 
+
     describe('enforce no todo', () => {
         it('default todo pattern', async () => {
             const diagnostics = await linter.run({
@@ -534,6 +535,36 @@ describe('codeStyle', () => {
         });
     });
 
+    it('enforce no assocarray component field type', async () => {
+        const diagnostics = await linter.run({
+            ...project1,
+            files: ['components/interfaceTest.xml'],
+            rules: {
+                'no-assocarray-component-field-type': 'info'
+            }
+        } as any);
+        const actual = fmtDiagnostics(diagnostics);
+        const expected = [
+            `06:LINT3024:Avoid using field type 'assocarray'`
+        ];
+        expect(actual).deep.equal(expected);
+    });
+
+    it('enforce no array component field type', async () => {
+        const diagnostics = await linter.run({
+            ...project1,
+            files: ['components/interfaceTest.xml'],
+            rules: {
+                'no-array-component-field-type': 'info'
+            }
+        } as any);
+        const actual = fmtDiagnostics(diagnostics);
+        const expected = [
+            `04:LINT3025:Avoid using field type 'array'`
+        ];
+        expect(actual).deep.equal(expected);
+    });
+
     describe('AA style', () => {
         it('collects wrapping AA members indexes', () => {
             const { statements } = Parser.parse(`

src/plugins/codeStyle/index.ts

@@ -1,4 +1,22 @@
-import { BscFile, BsDiagnostic, createVisitor, FunctionExpression, isBrsFile, isGroupingExpression, TokenKind, WalkMode, CancellationTokenSource, DiagnosticSeverity, OnGetCodeActionsEvent, isCommentStatement, AALiteralExpression, AAMemberExpression } from 'brighterscript';
+import {
+    BscFile,
+    XmlFile,
+    BsDiagnostic,
+    createVisitor,
+    FunctionExpression,
+    isBrsFile,
+    isXmlFile,
+    isGroupingExpression,
+    TokenKind,
+    WalkMode,
+    CancellationTokenSource,
+    DiagnosticSeverity,
+    OnGetCodeActionsEvent,
+    isCommentStatement,
+    AALiteralExpression,
+    AAMemberExpression,
+    BrsFile
+} from 'brighterscript';
 import { RuleAAComma } from '../..';
 import { addFixesToEvent } from '../../textEdit';
 import { PluginContext } from '../../util';
@@ -18,13 +36,43 @@ export default class CodeStyle {
         extractFixes(addFixes, event.diagnostics);
     }
 
-    afterFileValidate(file: BscFile) {
-        if (!isBrsFile(file) || this.lintContext.ignores(file)) {
-            return;
+    validateXMLFile(file: XmlFile) {
+        const diagnostics: Omit<BsDiagnostic, 'file'>[] = [];
+        const { noArrayComponentFieldType, noAssocarrayComponentFieldType } = this.lintContext.severity;
+
+        const validateArrayComponentFieldType = noArrayComponentFieldType !== DiagnosticSeverity.Hint;
+        const validateAssocarrayComponentFieldType = noAssocarrayComponentFieldType !== DiagnosticSeverity.Hint;
+
+        for (const field of file.parser?.ast?.component?.api?.fields ?? []) {
+            const { tag, attributes } = field;
+            if (tag.text === 'field') {
+                const typeAttribute = attributes.find(({ key }) => key.text === 'type');
+
+                const typeValue = typeAttribute?.value.text;
+                if (typeValue === 'array' && validateArrayComponentFieldType) {
+                    diagnostics.push(
+                        messages.noArrayFieldType(
+                            typeAttribute.value.range,
+                            noArrayComponentFieldType
+                        )
+                    );
+                } else if (typeValue === 'assocarray' && validateAssocarrayComponentFieldType) {
+                    diagnostics.push(
+                        messages.noAssocarrayFieldType(
+                            typeAttribute.value.range,
+                            noAssocarrayComponentFieldType
+                        )
+                    );
+                }
+            }
         }
 
+        return diagnostics;
+    }
+
+    validateBrsFile(file: BrsFile) {
         const diagnostics: (Omit<BsDiagnostic, 'file'>)[] = [];
-        const { severity, fix } = this.lintContext;
+        const { severity } = this.lintContext;
         const { inlineIfStyle, blockIfStyle, conditionStyle, noPrint, noTodo, noStop, aaCommaStyle, eolLast, colorFormat } = severity;
         const validatePrint = noPrint !== DiagnosticSeverity.Hint;
         const validateTodo = noTodo !== DiagnosticSeverity.Hint;
@@ -163,12 +211,29 @@ export default class CodeStyle {
             this.validateFunctionStyle(fun, diagnostics);
         }
 
+        return diagnostics;
+    }
+
+    afterFileValidate(file: BscFile) {
+        if (this.lintContext.ignores(file)) {
+            return;
+        }
+
+        const diagnostics: (Omit<BsDiagnostic, 'file'>)[] = [];
+        if (isXmlFile(file)) {
+            diagnostics.push(...this.validateXMLFile(file));
+        } else if (isBrsFile(file)) {
+            diagnostics.push(...this.validateBrsFile(file));
+        }
+
         // add file reference
         let bsDiagnostics: BsDiagnostic[] = diagnostics.map(diagnostic => ({
             ...diagnostic,
             file
         }));
 
+        const { fix } = this.lintContext;
+
         // apply fix
         if (fix) {
             bsDiagnostics = extractFixes(this.lintContext.addFixes, bsDiagnostics);

src/util.ts

@@ -26,7 +26,9 @@ export function getDefaultRules(): BsLintConfig['rules'] {
         'anon-function-style': 'auto',
         'aa-comma-style': 'no-dangling',
         'type-annotations': 'off',
-        'no-print': 'off'
+        'no-print': 'off',
+        'no-assocarray-component-field-type': 'off',
+        'no-array-component-field-type': 'off'
     };
 }
 
@@ -162,7 +164,9 @@ function rulesToSeverity(rules: BsLintConfig['rules']) {
         colorCase: rules['color-case'],
         colorAlpha: rules['color-alpha'],
         colorAlphaDefaults: rules['color-alpha-defaults'],
-        colorCertCompliant: rules['color-cert']
+        colorCertCompliant: rules['color-cert'],
+        noAssocarrayComponentFieldType: ruleToSeverity(rules['no-assocarray-component-field-type']),
+        noArrayComponentFieldType: ruleToSeverity(rules['no-array-component-field-type'])
     };
 }
 

test/project1/components/interfaceTest.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<component name="interfaceTest" extends="Group">
+    <interface>
+        <field id="test" type="array" />
+        <field id="test2" type="node" />
+        <field id="test3" type="assocarray" />
+    </interface>
+</component>