Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/DiagnosticFilterer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,57 @@ describe('DiagnosticFilterer', () => {
});
});

describe('getPathLikeDiagnosticFilterCodes', () => {
it('flags entries with `./`', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: ['./source/main.brs']
})).to.eql(['./source/main.brs']);
});

it('flags entries with `../`', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: ['../vendor/lib.bs']
})).to.eql(['../vendor/lib.bs']);
});

it('flags entries with a globstar', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: ['source/vendor/**/*']
})).to.eql(['source/vendor/**/*']);
});

it('flags entries ending in .bs, .brs, or .xml', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: ['source/main.brs', 'source/lib.bs', 'components/Widget.xml']
})).to.eql(['source/main.brs', 'source/lib.bs', 'components/Widget.xml']);
});

it('flags path-like codes nested in a `codes` array', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: [{ codes: [1, 'lint-1000', 'source/vendor/**/*'] }]
})).to.eql(['source/vendor/**/*']);
});

it('does not flag actual diagnostic codes', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: [1000, 'lint-1000', { codes: [1, 'lint-1000'] }, { files: 'source/vendor/**/*' }]
})).to.eql([]);
});

it('does not flag anything when diagnosticFiltersV0Compatibility is true', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFiltersV0Compatibility: true,
diagnosticFilters: ['source/vendor/**/*']
} as any)).to.eql([]);
});

it('does not duplicate repeated offenders', () => {
expect(filterer.getPathLikeDiagnosticFilterCodes({
diagnosticFilters: ['source/vendor/**/*', 'source/vendor/**/*']
})).to.eql(['source/vendor/**/*']);
});
});

});

function getDiagnostic(code: number | string, srcPath: string, destPath?: string) {
Expand Down
44 changes: 44 additions & 0 deletions src/DiagnosticFilterer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,48 @@ export class DiagnosticFilterer {
}

}

/**
* Does this diagnostic filter "code" value look like a file path/glob instead of a diagnostic code?
* v0-style `diagnosticFilters` entries were file globs, but v1 treats bare string/number entries as codes.
* This heuristic catches the common glob patterns left over from a v0-style config.
*/
public isCodeValuePathLike(value: number | string): boolean {
if (typeof value !== 'string') {
return false;
}
const lowerValue = value.toLowerCase();
return (
value.includes('./') ||

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about windows? .\

value.includes('**') ||
lowerValue.endsWith('.bs') ||
lowerValue.endsWith('.brs') ||
lowerValue.endsWith('.xml')
);
}

/**
* Scan the (non-v0-compat) `diagnosticFilters` config for entries that look like file paths/globs
* rather than diagnostic codes, to help teams migrating from the v0-style config.
*/
public getPathLikeDiagnosticFilterCodes(config: BsConfig): (number | string)[] {
if (config.diagnosticFiltersV0Compatibility) {
return [];
}
const result = new Set<number | string>();
for (let filter of config.diagnosticFilters ?? []) {
if ((typeof filter === 'string' || typeof filter === 'number') && this.isCodeValuePathLike(filter)) {
result.add(filter);
continue;
}
if (filter && typeof filter === 'object' && 'codes' in filter && Array.isArray(filter.codes)) {
for (const code of filter.codes) {
if (this.isCodeValuePathLike(code)) {
result.add(code);
}
}
}
}
return [...result];
}
}
22 changes: 21 additions & 1 deletion src/DiagnosticManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import type { Logger } from './logging';
import { LogLevel, createLogger } from './logging';
import type { Program } from './Program';
import type { BrsFile } from './files/BrsFile';
import { DiagnosticCodeMap } from './DiagnosticMessages';
import { DiagnosticCodeMap, DiagnosticMessages } from './DiagnosticMessages';
import * as path from 'path';

interface DiagnosticWithContexts {
diagnostic: BsDiagnosticWithKey;
Expand Down Expand Up @@ -489,6 +490,25 @@ export class DiagnosticManager {
}
return this.diagnosticFilterer.isFileCompletelyFiltered(file);
}

/**
* Flag `diagnosticFilters` entries that look like file paths/globs rather than diagnostic codes.
* This is a common mistake when migrating a bsconfig.json from the v0-style filters (which were file globs)
*/
public detectPathLikeDiagnosticFilterCodes(config: FinalizedBsConfig, context?: DiagnosticContext) {
const pathLikeCodes = this.diagnosticFilterer.getPathLikeDiagnosticFilterCodes(config);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth checking to see if the pattern is exactly a path to a known file in the files array? like source/main.json or something?

if (pathLikeCodes.length === 0) {
return;
}
const location = util.createLocationFromRange(
util.pathToUri(config.project ?? path.join(config.cwd, 'bsconfig.json')),
util.createRange(0, 0, 0, 0)
);
this.register(pathLikeCodes.map(code => ({
...DiagnosticMessages.diagnosticFilterLooksLikeFilePath(code.toString()),
location: location
})), context);
}
}

interface DiagnosticContextFilter {
Expand Down
5 changes: 5 additions & 0 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,11 @@ export let DiagnosticMessages = {
legacyCode: 1154,
severity: DiagnosticSeverity.Error,
code: 'rsg-version-removed'
}),
diagnosticFilterLooksLikeFilePath: (value: string) => ({
message: `Diagnostic filter "${value}" looks like a file path or glob, not a diagnostic code. To filter diagnostics by file, use the "files" property instead: { "files": ["${value}"] }`,
severity: DiagnosticSeverity.Warning,
code: 'diagnostic-filter-looks-like-file-path'
})
};
export const defaultMaximumTruncationLength = 160;
Expand Down
14 changes: 14 additions & 0 deletions src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,20 @@ describe('Program', () => {
}]);
});

it('flags diagnosticFilters entries that look like file paths', () => {
program.options.diagnosticFilters = ['source/vendor/**/*'] as any;
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.diagnosticFilterLooksLikeFilePath('source/vendor/**/*')
]);
});

it('does not flag diagnosticFilters entries that are actual codes', () => {
program.options.diagnosticFilters = [1000, 'lint-1000'] as any;
program.validate();
expectDiagnostics(program, []);
});

it('does not produce duplicate parse errors for different component scopes', () => {
//add a file with a parse error
program.setFile('components/lib.brs', `
Expand Down
3 changes: 3 additions & 0 deletions src/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,9 @@ export class Program {
this.detectDuplicateComponentNames();


})
.once('detect diagnostic filter issues', () => {
this.diagnostics.detectPathLikeDiagnosticFilterCodes(this.options, { tags: [ProgramValidatorDiagnosticsTag] });
})
.onCancel(() => {
logValidateEnd('cancelled');
Expand Down
Loading