Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine Diagnostic Filters and adds human-readable diagnostic names #1241

Open
wants to merge 29 commits into
base: release-1.0.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e89872b
Took a stab at re-writing the docs
markwpearce Jun 26, 2024
c7c720f
Added human readable names to diagnostics
markwpearce Jun 27, 2024
656804f
Added file.destPath diagnostics filtering
markwpearce Jun 28, 2024
db4261c
hopefully fixed test
markwpearce Jun 28, 2024
ce50e12
Is this change needed too?
markwpearce Jun 28, 2024
647d89d
Changed BsDiagnostic so code is the human-readable version, legacyCod…
markwpearce Jun 28, 2024
75798ac
Fixed some issues
markwpearce Jun 28, 2024
0691af3
Fix misspelling
markwpearce Jun 28, 2024
ed3e9ec
Update docs/bsconfig.md
markwpearce Jul 12, 2024
34ae893
Update docs/bsconfig.md
markwpearce Jul 12, 2024
db7dfed
Updated docs to use new codes instead of legacy codes for diagnotsics
markwpearce Jul 12, 2024
ede4b5a
Update src/DiagnosticMessages.ts
markwpearce Jul 20, 2024
1713b97
Update src/DiagnosticMessages.ts
markwpearce Jul 20, 2024
896953d
Update src/DiagnosticMessages.ts
markwpearce Jul 20, 2024
5704458
Merge branch 'release-1.0.0' into refine_diagnostic_filters
markwpearce Jul 25, 2024
f2fa505
Changes from comments on PR
markwpearce Jul 25, 2024
dc65ad8
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
cd71343
Combined diagnostics to make new expectedOperator diagnostic
markwpearce Aug 20, 2024
02b445d
Merge branch 'release-1.0.0' into refine_diagnostic_filters
markwpearce Aug 20, 2024
e246efd
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
4ea0a52
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
c68d0d2
Combined expectedTerminator diags - Ill Be Back
markwpearce Aug 20, 2024
3c99174
Consolidated expected statement diagnostics
markwpearce Aug 20, 2024
21eb250
Removed xml prefix on xml diagnostics
markwpearce Aug 20, 2024
9174b6a
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
254cbe5
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
97f58a4
Update src/DiagnosticMessages.ts
markwpearce Aug 20, 2024
3658271
use one expected-identifier diaganostic code
markwpearce Aug 20, 2024
5dd9246
Consolidated 'expected terminator' diagnostics
markwpearce Oct 3, 2024
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
44 changes: 33 additions & 11 deletions docs/bsconfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,44 @@ If `true`, after a successful build, the project will be deployed to the Roku sp

## `diagnosticFilters`

Type: `Array<string | number | {src: string; codes: number[]}`
Type: `Array<string | number | {files: string | Array<string | {src: string} | {dest: string}>; codes?: Array<number | string>}`

A list of filters used to hide diagnostics.

- A `string` value should be a relative-to-root-dir or absolute file path or glob pattern of the files that should be excluded. Any file matching this pattern will have all diagnostics supressed. These file paths refer to the location of the source files pre-compilation and are relative to [`rootDir`](#rootdir). Absolute file paths may be used as well.
- A file glob may be prefixed with `!` to make it a negative pattern which "un-ignores" the files it matches. (See examples below).
- A `number` value should be a diagnostic code. This will supress all diagnostics with that code for the whole project.
- An object can also be provided to filter specific diagnostic codes for a file pattern. For example,
- A `string` value should be a diagnostic code, or diagnostic shortname. This will supress all diagnostics with that code for the whole project. For example,
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
```jsonc
"diagnosticFilters": [
"1000",
"1011",
"BS1001"
"mismatch-argument-count"
]
```

- A `number` value is treated as if it were a string, and matches all diagnostic codes that are the string representation of that number. For example, an entry of `1234` would suppress any diagnostic with code `'1234'`.
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
- An object can also be provided to filter specific diagnostic codes for a file pattern. If no `files` property is included, any diagnostic that matches the values in the `codes` will b e suppressed. If a `string` if given for the `files` property, it is treated as a relative-to-root-dir or absolute file path or glob pattern of the files that should be excluded. These file paths refer to the location of the source files pre-compilation and are relative to [`rootDir`](#rootdir). Absolute file paths may be used as well. For example,

```jsonc
"diagnosticFilters": [{
"files": "vendor/**/*",
"codes": ["1000", "1011"] //ignore these specific codes from vendor libraries
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
}]
```
- If an object is provided, the `files` property could also be an array, providing either a series of globs, or a specific set of files that match _either_ their `src` or `dest` paths. For example,

```jsonc
"diagnosticFilters": [{
"src": "vendor/**/*",
"codes": [1000, 1011] //ignore these specific codes from vendor libraries
"files": [
"vendor/**/*", // all vendor files will be suppressed
{ "src": "themes/theme1/**/*"}, // all files coming from `themes/theme1/` will be suppressed
{ "dest": "source/common/**/*"}, // all files then will be placed in from `source/common/` will be suppressed
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
]
"codes": ["1000", "1011"] //ignore these specific codes
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
}]
```

- A file glob may be prefixed with `!` to make it a negative pattern which "un-ignores" the files it matches. (See examples below).

Defaults to `undefined`.

If a child bsconfig extends from a parent bsconfig, and both bsconfigs specify `diagnosticFilters`, the parent bsconfig's `diagnosticFilters` field will be completely overwritten.
Expand All @@ -96,17 +118,17 @@ A negative pattern can be used to un-ignore some files or codes which were previ

```jsonc
"diagnosticFilters": [
{ "src": "vendor/**/*" }, //ignore all codes from vendor libraries
{ "src": "!vendor/unreliable/**/*" } //EXCEPT do show errors from this one specific library
{ "files": "vendor/**/*" }, //ignore all codes from vendor libraries
{ "files": "!vendor/unreliable/**/*" } //EXCEPT do show errors from this one specific library
]
```

A specific error code can be unignored in multiple places by using a pattern which matches everything under `rootDir`. For example,

```jsonc
"diagnosticFilters": [
{ "src": "vendor/**/*" }, //ignore all errors from vendor libraries
{ "src": "!*/**/*", "codes": [1000] } //EXCEPT do show this particular code everywhere
{ "files": "vendor/**/*" }, //ignore all errors from vendor libraries
{ "files": "!*/**/*", "codes": [1000] } //EXCEPT do show this particular code everywhere
markwpearce marked this conversation as resolved.
Show resolved Hide resolved
]
```

Expand Down
2 changes: 1 addition & 1 deletion src/BsConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export interface BsConfig {
/**
* A list of filters used to exclude diagnostics from the output
*/
diagnosticFilters?: Array<number | string | { src: string; codes: (number | string)[] } | { src: string } | { codes: (number | string)[] }>;
diagnosticFilters?: Array<string | number | { files?: string | Array<string | { src: string } | { dest: string }>; codes?: Array<number | string> }>;

/**
* Specify what diagnostic types should be printed to the console. Defaults to 'warn'
Expand Down
108 changes: 82 additions & 26 deletions src/DiagnosticFilterer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ describe('DiagnosticFilterer', () => {
let options = {
rootDir: rootDir,
diagnosticFilters: [
'codename',
//ignore these codes globally
{ codes: [1, 2, 3, 'X4'] },
//ignore all codes from lib
{ src: 'lib/**/*.brs' },
{ files: 'lib/**/*.brs' },
//ignore all codes from `packages` with absolute path
{ src: `${rootDir}/packages/**/*.brs` },
{ files: `${rootDir}/packages/**/*.brs` },
//ignore specific codes for main.brs
{ src: 'source/main.brs', codes: [4] }
{ files: 'source/main.brs', codes: [4] }
]
};

Expand Down Expand Up @@ -86,24 +87,34 @@ describe('DiagnosticFilterer', () => {
).to.eql([11, 12, 13, 'X14']);
});


it('works with single file src glob', () => {
expect(
filterer.filter(options, [
getDiagnostic('codename', `${rootDir}/source/main.brs`) //remove
]).map(x => x.code)
).to.eql([]);
});

describe('with negative globs', () => {
let optionsWithNegatives = {
rootDir: rootDir,
diagnosticFilters: [
//ignore these codes globally
{ codes: [1, 2] },
{ codes: [1, 2, 'codename'] },
3,
4,
'codename',
//ignore all codes from lib
{ src: 'lib/**/*.brs' },
{ files: 'lib/**/*.brs' },
//un-ignore specific errors from lib/special
{ src: '!lib/special/**/*.brs', codes: [1, 2, 3] },
{ files: '!lib/special/**/*.brs', codes: [1, 2, 3, 'codename'] },
//re-ignore errors from one extra special file
{ src: 'lib/special/all-reignored.brs' },
{ files: 'lib/special/all-reignored.brs' },
//un-ignore all codes from third special file
{ src: '!lib/special/all-unignored.brs' },
{ files: '!lib/special/all-unignored.brs' },
//un-ignore code 5 globally
{ src: '!*/**/*', codes: [5] },
{ files: '!*/**/*', codes: [5] },
//re-ignore code 10 globally, overriding previous unignores
{ codes: [10] }
]
Expand Down Expand Up @@ -180,15 +191,15 @@ describe('DiagnosticFilterer', () => {
it('handles standard diagnostic filters', () => {
expect(
filterer.getDiagnosticFilters({
diagnosticFilters: [{ src: 'file.brs', codes: [1, 2, 'X3'] }]
diagnosticFilters: [{ files: 'file.brs', codes: [1, 2, 'X3'] }]
})
).to.eql([{ src: 'file.brs', codes: [1, 2, 'X3'], isNegative: false }]);
});

it('handles string-only diagnostic filter object', () => {
expect(
filterer.getDiagnosticFilters({
diagnosticFilters: [{ src: 'file.brs' }]
diagnosticFilters: [{ files: 'file.brs' }]
})
).to.eql([{ src: 'file.brs', isNegative: false }]);
});
Expand All @@ -204,9 +215,9 @@ describe('DiagnosticFilterer', () => {
it('handles string diagnostic filter', () => {
expect(
filterer.getDiagnosticFilters({
diagnosticFilters: ['file.brs']
diagnosticFilters: ['cannot-find-name']
})
).to.eql([{ src: 'file.brs', isNegative: false }]);
).to.eql([{ codes: ['cannot-find-name'], isNegative: false }]);
});

it('converts ignoreErrorCodes to diagnosticFilters', () => {
Expand All @@ -217,19 +228,11 @@ describe('DiagnosticFilterer', () => {
]);
});

it('handles negative globs in bare strings', () => {
expect(filterer.getDiagnosticFilters({
diagnosticFilters: ['!file.brs']
})).to.eql([
{ src: 'file.brs', isNegative: true }
]);
});

it('handles negative globs in objects', () => {
expect(filterer.getDiagnosticFilters({
diagnosticFilters: [
{
src: '!file.brs'
files: '!file.brs'
}
]
})).to.eql([
Expand All @@ -241,7 +244,7 @@ describe('DiagnosticFilterer', () => {
expect(filterer.getDiagnosticFilters({
diagnosticFilters: [
{
src: '!file.brs',
files: '!file.brs',
codes: [1, 2, 3]
}
]
Expand All @@ -251,6 +254,49 @@ describe('DiagnosticFilterer', () => {
});
});

describe('filtering by dest', () => {
it('will filter by a files destPath', () => {
const resultDiagnostics = filterer.filter({
rootDir: rootDir,
diagnosticFilters: [
{
files: [
{ dest: 'source/remove/**/*.*' }
],
codes: ['diagCode']
}
]
}, [
getDiagnostic('diagCode', `${rootDir}/source/common.brs`), //keep
getDiagnostic('diagCode', `${rootDir}/source/utils.brs`, `${rootDir}/source/remove/utils.brs`), //remove
getDiagnostic('diagCode', `${rootDir}/components/utils.brs`, `${rootDir}/source/remove/utils2.brs`) //remove
]);
expect(resultDiagnostics.map(x => x.code)).to.eql(['diagCode']);
expect(resultDiagnostics.map(x => x.file.srcPath)).to.eql([s`${rootDir}/source/common.brs`]);
});

it('respects order of ignores with negative globs', () => {
const resultDiagnostics = filterer.filter({
rootDir: rootDir,
diagnosticFilters: [{
files: [
{ dest: 'source/**/*.*' } //ignore diagCode in files with destPath in /source/remove
],
codes: ['diagCode']
}, {
files: '!**/*.*', //unignore diagCode everywhere
codes: ['diagCode']
}
]
}, [
getDiagnostic('diagCode', `${rootDir}/source/common.brs`), //keep
getDiagnostic('diagCode', `${rootDir}/source/utils.brs`, `${rootDir}/source/remove/utils.brs`), //remove
getDiagnostic('diagCode', `${rootDir}/components/utils.brs`, `${rootDir}/source/remove/utils2.brs`) //remove
]);
expect(resultDiagnostics.map(x => x.code)).to.eql(['diagCode', 'diagCode', 'diagCode']);
});
});

it('only filters by file once per unique file (case-insensitive)', () => {
const stub = sinon.stub(filterer as any, 'filterFile').returns(null);
filterer.filter(options, [
Expand All @@ -259,19 +305,29 @@ describe('DiagnosticFilterer', () => {
getDiagnostic(3, s`${rootDir}/source/common2.brs`),
getDiagnostic(4, s`${rootDir}/source/Common2.brs`)
]);
expect(stub.callCount).to.eql(2);
const expectedCallCount = options.diagnosticFilters.reduce((acc, filter) => {
if (typeof filter === 'object' && 'files' in (filter as any)) {
return acc;
}
return acc + 1;
}, 0);
expect(stub.callCount).to.eql(expectedCallCount * 2); // 2 times for 'codename', 2 times for { codes: [1, 2, 3, 'X4'] }
expect(stub.getCalls().map(x => x.args[1])).to.eql([
s`${rootDir.toLowerCase()}/source/common1.brs`,
s`${rootDir.toLowerCase()}/source/common2.brs`,
s`${rootDir.toLowerCase()}/source/common1.brs`,
s`${rootDir.toLowerCase()}/source/common2.brs`
]);
});

});

function getDiagnostic(code: number | string, srcPath: string) {
function getDiagnostic(code: number | string, srcPath: string, destPath?: string) {
destPath = destPath ?? srcPath;
return {
file: {
srcPath: s`${srcPath}`
srcPath: s`${srcPath}`,
destPath: s`${destPath}`
},
code: code
} as BsDiagnostic;
Expand Down
Loading
Loading