• Home
  • Features
  • Pricing
  • Docs
  • Announcements
  • Sign In

rokucommunity / bslint / #1039

03 Oct 2024 07:42PM CUT coverage: 91.231% (-0.5%) from 91.746%
#1039

Pull #96

TwitchBronBron
1.0.0-alpha.39
Pull Request #96: v1

927 of 1061 branches covered (87.37%)

Branch coverage included in aggregate %.

231 of 240 new or added lines in 12 files covered. (96.25%)

11 existing lines in 3 files now uncovered.

1008 of 1060 relevant lines covered (95.09%)

68.84 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

92.16
/src/plugins/trackCodeFlow/varTracking.ts
1
import { BscFile, FunctionExpression, BsDiagnostic, Range, isForStatement, isForEachStatement, isIfStatement, isAssignmentStatement, isNamespaceStatement, NamespaceStatement, Expression, isVariableExpression, isBinaryExpression, TokenKind, Scope, CallableContainerMap, DiagnosticSeverity, isLiteralInvalid, isWhileStatement, isCatchStatement, isLabelStatement, isGotoStatement, ParseMode, util, isMethodStatement, isTryCatchStatement, isConditionalCompileStatement, VariableExpression } from 'brighterscript';
1✔
2
import { LintState, StatementInfo, NarrowingInfo, VarInfo, VarRestriction } from '.';
1✔
3
import { PluginContext } from '../../util';
4
import { Location } from 'vscode-languageserver-types';
1✔
5

6
export enum VarLintError {
1✔
7
    UninitializedVar = 'LINT1001',
1✔
8
    UnsafeIteratorVar = 'LINT1002',
1✔
9
    UnsafeInitialization = 'LINT1003',
1✔
10
    CaseMismatch = 'LINT1004',
1✔
11
    UnusedVariable = 'LINT1005'
1✔
12
}
13

14
enum ValidationKind {
1✔
15
    Assignment = 'Assignment',
1✔
16
    UninitializedVar = 'UninitializedVar',
1✔
17
    Unsafe = 'Unsafe'
1✔
18
}
19

20

21
export const VarTrackingMessages = {
1✔
22
    varCasing: (curr: VarInfo, name: { text: string; location: Location }) => ({
12✔
23
        severity: DiagnosticSeverity.Warning,
24
        source: 'bslint',
25
        code: VarLintError.CaseMismatch,
26
        message: `Variable '${name.text}' was previously set with a different casing as '${curr.name}'`,
27
        data: {
28
            name: curr.name,
29
            location: name.location
30
        }
31
    }),
32
    unsafeIterator: (name: string) => ({
2✔
33
        severity: DiagnosticSeverity.Error,
34
        source: 'bslint',
35
        code: VarLintError.UnsafeIteratorVar,
36
        message: `Using iterator variable '${name}' outside loop`
37
    }),
38
    unusedVariable: (name: string) => ({
44✔
39
        severity: DiagnosticSeverity.Warning,
40
        source: 'bslint',
41
        code: VarLintError.UnusedVariable,
42
        message: `Variable '${name}' is set but value is never used`
43
    }),
44
    unsafeInitialization: (name: string) => ({
19✔
45
        severity: DiagnosticSeverity.Error,
46
        source: 'bslint',
47
        code: VarLintError.UnsafeInitialization,
48
        message: `Not all the code paths assign '${name}'`
49
    }),
50
    uninitializedVariable: (name: string, scopeName: string) => ({
7✔
51
        severity: DiagnosticSeverity.Error,
52
        source: 'bslint',
53
        code: VarLintError.UninitializedVar,
54
        message: `Using uninitialised variable '${name}' when this file is included in scope '${scopeName}'`
55
    })
56
};
57

58

59
interface ValidationInfo {
60
    kind: ValidationKind;
61
    name: string;
62
    local?: VarInfo;
63
    location: Location;
64
    namespace?: NamespaceStatement;
65
}
66

67
const deferredValidation: Map<string, ValidationInfo[]> = new Map();
1✔
68

69
function getDeferred(file: BscFile) {
70
    return deferredValidation.get(file.srcPath);
145✔
71
}
72

73
export function resetVarContext(file: BscFile) {
1✔
74
    deferredValidation.set(file.srcPath, []);
39✔
75
}
76

77
export function createVarLinter(
1✔
78
    lintContext: PluginContext,
79
    file: BscFile,
80
    fun: FunctionExpression,
81
    state: LintState,
82
    diagnostics: BsDiagnostic[]
83
) {
84
    const { severity } = lintContext;
145✔
85
    const deferred = getDeferred(file);
145✔
86
    let foundLabelAt = 0;
145✔
87

88
    const args: Map<string, VarInfo> = new Map();
145✔
89
    args.set('m', { name: 'm', location: Location.create('', Range.create(0, 0, 0, 0)), isParam: true, isUnsafe: false, isUsed: true });
145✔
90
    fun.parameters.forEach((p) => {
145✔
91
        const name = p.tokens.name.text;
11✔
92
        args.set(name.toLowerCase(), { name: name, location: p.tokens.name.location, isParam: true, isUnsafe: false, isUsed: false });
11✔
93
    });
94

95
    if (isMethodStatement(fun.parent)) {
145✔
96
        args.set('super', { name: 'super', location: null, isParam: true, isUnsafe: false, isUsed: true });
8✔
97
    }
98

99
    function verifyVarCasing(curr: VarInfo, name: { text: string; location: Location }) {
100
        if (curr && curr.name !== name.text) {
255✔
101
            diagnostics.push({
12✔
102
                ...VarTrackingMessages.varCasing(curr, name),
103
                severity: severity.caseSensitivity,
104
                location: name.location
105
            });
106
        }
107
    }
108

109
    function setLocal(parent: StatementInfo, name: { text: string; location: Location }, restriction?: VarRestriction): VarInfo {
110
        if (!name) {
228!
111
            return;
×
112
        }
113
        const key = name.text.toLowerCase();
228✔
114
        const arg = args.get(key);
228✔
115
        const local = {
228✔
116
            name: name.text,
117
            location: name.location,
118
            parent: parent,
119
            restriction: restriction,
120
            metBranches: 1,
121
            isUnsafe: false,
122
            isUsed: false
123
        };
124
        if (arg) {
228✔
125
            verifyVarCasing(arg, name);
3✔
126
            return local;
3✔
127
        }
128

129
        if (!parent.locals) {
225✔
130
            parent.locals = new Map();
161✔
131
        } else {
132
            verifyVarCasing(parent.locals.get(key), name);
64✔
133
        }
134
        parent.locals.set(key, local);
225✔
135

136
        deferred.push({
225✔
137
            kind: ValidationKind.Assignment,
138
            name: name.text,
139
            local: local,
140
            location: name.location
141
        });
142

143
        return local;
225✔
144
    }
145

146
    function findLocal(name: string): VarInfo | undefined {
147
        const key = name.toLowerCase();
263✔
148
        const arg = args.get(key);
263✔
149
        if (arg) {
263✔
150
            return arg;
9✔
151
        }
152
        const { parent, blocks, stack } = state;
254✔
153

154
        if (parent?.locals?.has(key)) {
254!
155
            return parent.locals.get(key);
125✔
156
        }
157
        for (let i = stack.length - 2; i >= 0; i--) {
129✔
158
            const block = blocks.get(stack[i]);
102✔
159
            const local = block?.locals?.get(key);
102!
160
            if (local) {
102✔
161
                return local;
56✔
162
            }
163
        }
164
        return undefined;
73✔
165
    }
166

167
    // A local was found but it is considered unsafe (e.g. set in an if branch)
168
    // Found out whether a parent has this variable set safely
169
    function findSafeLocal(name: string): VarInfo | undefined {
170
        const key = name.toLowerCase();
33✔
171
        const { blocks, stack } = state;
33✔
172
        if (stack.length < 2) {
33✔
173
            return undefined;
24✔
174
        }
175
        for (let i = stack.length - 2; i >= 0; i--) {
9✔
176
            const block = blocks.get(stack[i]);
18✔
177
            const local = block?.locals?.get(key);
18!
178
            // if partial, look up higher in the scope for a non-partial
179
            if (local && !local.isUnsafe) {
18✔
180
                return local;
2✔
181
            }
182
        }
183
    }
184

185
    function openBlock(block: StatementInfo) {
186
        const { stat } = block;
393✔
187
        if (isForStatement(stat)) {
393✔
188
            // for iterator will be declared by the next assignement statement
189
        } else if (isForEachStatement(stat)) {
386✔
190
            // declare `for each` iterator variable
191
            setLocal(block, stat.tokens.item, VarRestriction.Iterator);
4✔
192
        } else if (state.parent?.narrows) {
382✔
193
            narrowBlock(block);
10✔
194
        }
195
    }
196

197
    function narrowBlock(block: StatementInfo) {
198
        const { parent } = state;
10✔
199
        const { stat } = block;
10✔
200

201
        if (isIfStatement(stat) && isIfStatement(parent.stat)) {
10!
202
            block.narrows = parent?.narrows;
×
203
            return;
×
204
        }
205

206
        parent?.narrows?.forEach(narrow => {
10!
207
            if (narrow.block === stat) {
10✔
208
                setLocal(block, narrow).narrowed = narrow;
9✔
209
            } else {
210
                // opposite narrowing for other branches
211
                setLocal(block, narrow).narrowed = {
1✔
212
                    ...narrow,
213
                    type: narrow.type === 'invalid' ? 'valid' : 'invalid'
1!
214
                };
215
            }
216
        });
217
    }
218

219
    function visitStatement(curr: StatementInfo) {
220
        const { stat } = curr;
806✔
221
        if (isAssignmentStatement(stat) && state.parent) {
806✔
222
            // value = stat.value;
223
            setLocal(state.parent, stat.tokens.name, isForStatement(state.parent.stat) ? VarRestriction.Iterator : undefined);
211✔
224
        } else if (isCatchStatement(stat) && state.parent) {
595✔
225
            setLocal(curr, (stat.exceptionVariableExpression as VariableExpression)?.tokens?.name, VarRestriction.CatchedError);
3!
226
        } else if (isLabelStatement(stat) && !foundLabelAt) {
592✔
227
            foundLabelAt = stat.location.range.start.line;
1✔
228
        } else if (foundLabelAt && isGotoStatement(stat) && state.parent) {
591✔
229
            // To avoid false positives when finding a goto statement,
230
            // very generously mark as used all unused variables after 1st found label line.
231
            // This isn't accurate but tracking usage across goto jumps is tricky
232
            const { stack, blocks } = state;
1✔
233
            const labelLine = foundLabelAt;
1✔
234
            for (let i = state.stack.length - 1; i >= 0; i--) {
1✔
235
                const block = blocks.get(stack[i]);
3✔
236
                block?.locals?.forEach(local => {
3!
237
                    if (local.location.range?.start.line > labelLine) {
5!
238
                        local.isUsed = true;
4✔
239
                    }
240
                });
241
            }
242
        }
243
    }
244

245
    function closeBlock(closed: StatementInfo) {
246
        const { locals, branches, returns } = closed;
400✔
247
        const { parent } = state;
400✔
248
        if (!locals || !parent) {
400✔
249
            if (locals) {
229✔
250
                finalize(locals);
86✔
251
            }
252
            return;
229✔
253
        }
254
        // when closing a branched statement, evaluate vars with partial branches covered
255
        if (branches > 1) {
171✔
256
            locals.forEach((local) => {
75✔
257
                if (local.metBranches !== branches) {
79✔
258
                    local.isUnsafe = true;
61✔
259
                }
260
                local.metBranches = 1;
79✔
261
            });
262
        } else if (isIfStatement(parent.stat)) {
96✔
263
            locals.forEach(local => {
60✔
264
                // keep narrowed vars if we `return` the invalid branch
265
                if (local.narrowed) {
62✔
266
                    if (!returns || local.narrowed.type === 'valid') {
10✔
267
                        locals.delete(local.name.toLowerCase());
7✔
268
                    }
269
                }
270
            });
271
        } else if (isCatchStatement(closed.stat)) {
36✔
272
            locals.forEach(local => {
3✔
273
                // drop error variable
274
                if (local.restriction === VarRestriction.CatchedError) {
4✔
275
                    locals.delete(local.name.toLowerCase());
3✔
276
                }
277
            });
278
        }
279
        // move locals to parent
280
        if (!parent.locals) {
171✔
281
            parent.locals = locals;
96✔
282
        } else {
283
            const isParentBranched = isIfStatement(parent.stat) || isTryCatchStatement(parent.stat) || isConditionalCompileStatement(parent.stat);
75✔
284
            const isLoop = isForStatement(closed.stat) || isForEachStatement(closed.stat) || isWhileStatement(closed.stat);
75✔
285
            locals.forEach((local, name) => {
75✔
286
                const parentLocal = parent.locals.get(name);
76✔
287
                // if var is an iterator var, flag as partial
288
                if (local.restriction) {
76✔
289
                    local.isUnsafe = true;
8✔
290
                }
291
                // combine attributes / met branches
292
                if (isParentBranched) {
76✔
293
                    if (parentLocal) {
21✔
294
                        local.isUnsafe = parentLocal.isUnsafe || local.isUnsafe;
18✔
295
                        local.metBranches = (parentLocal.metBranches ?? 0) + 1;
18!
296
                    }
297
                } else if (parentLocal && !parentLocal.isUnsafe) {
55✔
298
                    local.isUnsafe = false;
14✔
299
                }
300
                if (parentLocal?.restriction) {
76!
301
                    local.restriction = parentLocal.restriction;
×
302
                }
303
                if (!local.isUsed && isLoop) {
76✔
304
                    // avoid false positive if a local set in a loop isn't used
305
                    const someParentLocal = findLocal(local.name);
3✔
306
                    if (someParentLocal?.isUsed) {
3✔
307
                        local.isUsed = true;
1✔
308
                    }
309
                }
310
                parent.locals.set(name, local);
76✔
311
            });
312
        }
313
    }
314

315
    function visitExpression(expr: Expression, parent: Expression, curr: StatementInfo) {
316
        if (isVariableExpression(expr) && !util.isInTypeExpression(expr)) {
931✔
317
            const name = expr.tokens.name.text;
263✔
318
            if (name === 'm') {
263✔
319
                return;
3✔
320
            }
321

322
            const local = findLocal(name);
260✔
323
            if (!local) {
260✔
324
                deferred.push({
72✔
325
                    kind: ValidationKind.UninitializedVar,
326
                    name: name,
327
                    location: expr.location,
328
                    namespace: expr.findAncestor<NamespaceStatement>(isNamespaceStatement)
329
                });
330
                return;
72✔
331
            } else {
332
                local.isUsed = true;
188✔
333
                verifyVarCasing(local, expr.tokens.name);
188✔
334
            }
335

336
            if (local.isUnsafe && !findSafeLocal(name)) {
188✔
337
                if (local.restriction) {
31✔
338
                    diagnostics.push({
2✔
339
                        ...VarTrackingMessages.unsafeIterator(name),
340
                        severity: severity.unsafeIterators,
341
                        location: expr.location
342
                    });
343
                } else if (!isNarrowing(local, expr, parent, curr)) {
29✔
344
                    diagnostics.push({
19✔
345
                        ...VarTrackingMessages.unsafeInitialization(name),
346
                        severity: severity.unsafePathLoop, // should this be severity.assignAllPath?
347
                        location: expr.location
348
                    });
349
                }
350
            }
351
        }
352
    }
353

354
    function isNarrowing(local: VarInfo, expr: Expression, parent: Expression, curr: StatementInfo): boolean {
355
        // Are we inside an `if/elseif` statement condition?
356
        if (!isIfStatement(curr.stat)) {
29✔
357
            return false;
19✔
358
        }
359
        const block = curr.stat.thenBranch;
10✔
360
        // look for a statement testing whether variable is `invalid`,
361
        // like `if x <> invalid` or `else if x = invalid`
362
        if (!isBinaryExpression(parent) || !(isLiteralInvalid(parent.left) || isLiteralInvalid(parent.right))) {
10✔
363
            // maybe the variable was narrowed as part of the condition
364
            // e.g. 2nd condition in: if x <> invalid and x.y = z
365
            return curr.narrows?.some(narrow => narrow.text === local.name);
1!
366
        }
367
        const operator = parent.tokens.operator.kind;
9✔
368
        if (operator !== TokenKind.Equal && operator !== TokenKind.LessGreater) {
9!
369
            return false;
×
370
        }
371
        const narrow: NarrowingInfo = {
9✔
372
            text: local.name,
373
            location: local.location,
374
            type: operator === TokenKind.Equal ? 'invalid' : 'valid',
9✔
375
            block
376
        };
377
        if (!curr.narrows) {
9!
378
            curr.narrows = [];
9✔
379
        }
380
        curr.narrows.push(narrow);
9✔
381
        return true;
9✔
382
    }
383

384
    function finalize(locals: Map<string, VarInfo>) {
385
        locals.forEach(local => {
86✔
386
            if (!local.isUsed && !local.restriction) {
154✔
387
                diagnostics.push({
44✔
388
                    ...VarTrackingMessages.unusedVariable(local.name),
389
                    severity: severity.unusedVariable,
390
                    location: local.location
391
                });
392
            }
393
        });
394
    }
395

396
    return {
145✔
397
        openBlock: openBlock,
398
        closeBlock: closeBlock,
399
        visitStatement: visitStatement,
400
        visitExpression: visitExpression
401
    };
402
}
403

404
export function runDeferredValidation(
1✔
405
    lintContext: PluginContext,
406
    scope: Scope,
407
    files: BscFile[],
408
    callables: CallableContainerMap
409
) {
410
    const topLevelVars = buildTopLevelVars(scope, lintContext.globals);
39✔
411
    const diagnostics: BsDiagnostic[] = [];
39✔
412
    files.forEach((file) => {
39✔
413
        const deferred = deferredValidation.get(file.srcPath);
39✔
414
        if (deferred) {
39!
415
            deferredVarLinter(scope, file, callables, topLevelVars, deferred, diagnostics);
39✔
416
        }
417
    });
418
    return diagnostics;
39✔
419
}
420

421
/**
422
 * Get a list of all top level variables available in the scope
423
 */
424
function buildTopLevelVars(scope: Scope, globals: string[]) {
425
    // lookups for namespaces, classes, enums, etc...
426
    // to add them to the topLevel so that they don't get marked as unused.
427
    const toplevel = new Set<string>(globals);
39✔
428

429
    for (const namespace of scope.getAllNamespaceStatements()) {
39✔
430
        toplevel.add(getRootNamespaceName(namespace).toLowerCase()); // keep root of namespace
6✔
431
    }
432
    for (const [, cls] of scope.getClassMap()) {
39✔
433
        toplevel.add(cls.item.tokens.name.text.toLowerCase());
5✔
434
    }
435
    for (const [, enm] of scope.getEnumMap()) {
39✔
436
        toplevel.add(enm.item.name.toLowerCase());
3✔
437
    }
438
    for (const [, cnst] of scope.getConstMap()) {
39✔
439
        toplevel.add(cnst.item.name.toLowerCase());
1✔
440
    }
441
    return toplevel;
39✔
442
}
443

444
function deferredVarLinter(
445
    scope: Scope,
446
    file: BscFile,
447
    callables: CallableContainerMap,
448
    toplevel: Set<string>,
449
    deferred: ValidationInfo[],
450
    diagnostics: BsDiagnostic[]
451
) {
452
    deferred.forEach(({ kind, name, local, location, namespace }) => {
39✔
453
        const key = name?.toLowerCase();
297!
454
        let hasCallable = key ? callables.has(key) || toplevel.has(key) : false;
297!
455
        if (key && !hasCallable && namespace) {
297✔
456
            // check if this could be a callable in the current namespace
457
            const keyUnderNamespace = `${namespace.getName(ParseMode.BrightScript)}_${key}`.toLowerCase();
3✔
458
            hasCallable = callables.has(keyUnderNamespace);
3✔
459
        }
460
        switch (kind) {
297!
461
            case ValidationKind.UninitializedVar:
462
                if (!hasCallable) {
72✔
463
                    diagnostics.push({
7✔
464
                        ...VarTrackingMessages.uninitializedVariable(name, scope.name),
465
                        severity: DiagnosticSeverity.Error,
466
                        location: location
467
                    });
468
                }
469
                // TODO else test case
470
                break;
72✔
471
            case ValidationKind.Assignment:
472
                break;
225✔
473
            case ValidationKind.Unsafe:
474
                break;
×
475
        }
476
    });
477
}
478

479
/**
480
 * Get the leftmost part of the namespace name. (i.e. `alpha` from `alpha.beta.charlie`) by walking
481
 * up the namespace chain until we get to the very topmost namespace. Then grabbing the leftmost token's name.
482
 *
483
 */
484
export function getRootNamespaceName(namespace: NamespaceStatement) {
1✔
485
    // there are more concise ways to accomplish this, but this is a hot function so it's been optimized.
486
    while (true) {
6✔
487
        const parent = namespace.parent?.parent as NamespaceStatement;
6!
488
        if (isNamespaceStatement(parent)) {
6!
489
            namespace = parent;
×
490
        } else {
491
            break;
6✔
492
        }
493
    }
494
    const result = util.getDottedGetPath(namespace.nameExpression)[0]?.tokens.name?.text;
6!
495
    // const name = namespace.getName(ParseMode.BrighterScript).toLowerCase();
496
    // if (name.includes('imigx')) {
497
    //     console.log([name, result]);
498
    // }
499
    return result;
6✔
500
}
STATUS · Troubleshooting · Open an Issue · Sales · Support · CAREERS · ENTERPRISE · START FREE · SCHEDULE DEMO
ANNOUNCEMENTS · TWITTER · TOS & SLA · Supported CI Services · What's a CI service? · Automated Testing

© 2025 Coveralls, Inc