Skip to content

Commit 19bb7df

Browse files
tonybaloneykarthiknadig
tonybaloney
and
karthiknadig
authored
Refactor LinterId from string union to an enum (#12116)
* Refactor LinterId from string union to an enum and update tests and usages. * Fix incorrect LINTERID_BY_PRODUCT entry and add news item * Update news/3 Code Health/12116.md Co-authored-by: Karthik Nadig <kanadig@microsoft.com> * Revert lockfile changes Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
1 parent 2016f0e commit 19bb7df

8 files changed

Lines changed: 41 additions & 32 deletions

File tree

‎news/3 Code Health/12116.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Refactor `LinterId` to an enum instead of a string union.
2+
(thanks to [Anthony Shaw](https://github.com/tonybaloney))

‎src/client/linters/constants.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import { LinterId } from './types';
88

99
// All supported linters must be in this map.
1010
export const LINTERID_BY_PRODUCT = new Map<Product, LinterId>([
11-
[Product.bandit, 'bandit'],
12-
[Product.flake8, 'flake8'],
13-
[Product.pylint, 'pylint'],
14-
[Product.mypy, 'mypy'],
15-
[Product.pycodestyle, 'pycodestyle'],
16-
[Product.prospector, 'prospector'],
17-
[Product.pydocstyle, 'pydocstyle'],
18-
[Product.pylama, 'pylama']
11+
[Product.bandit, LinterId.Bandit],
12+
[Product.flake8, LinterId.Flake8],
13+
[Product.pylint, LinterId.PyLint],
14+
[Product.mypy, LinterId.MyPy],
15+
[Product.pycodestyle, LinterId.PyCodeStyle],
16+
[Product.prospector, LinterId.Prospector],
17+
[Product.pydocstyle, LinterId.PyDocStyle],
18+
[Product.pylama, LinterId.PyLama]
1919
]);

‎src/client/linters/linterInfo.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export class PylintLinterInfo extends LinterInfo {
8383
private readonly workspaceService: IWorkspaceService,
8484
configFileNames: string[] = []
8585
) {
86-
super(Product.pylint, 'pylint', configService, configFileNames);
86+
super(Product.pylint, LinterId.PyLint, configService, configFileNames);
8787
}
8888
public isEnabled(resource?: Uri): boolean {
8989
// We want to be sure the setting is not default since default is `true` and hence

‎src/client/linters/linterManager.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ import { Pycodestyle } from './pycodestyle';
1818
import { PyDocStyle } from './pydocstyle';
1919
import { PyLama } from './pylama';
2020
import { Pylint } from './pylint';
21-
import { IAvailableLinterActivator, ILinter, ILinterInfo, ILinterManager, ILintMessage } from './types';
21+
import { IAvailableLinterActivator, ILinter, ILinterInfo, ILinterManager, ILintMessage, LinterId } from './types';
2222

2323
class DisabledLinter implements ILinter {
2424
constructor(private configService: IConfigurationService) {}
2525
public get info() {
26-
return new LinterInfo(Product.pylint, 'pylint', this.configService);
26+
return new LinterInfo(Product.pylint, LinterId.PyLint, this.configService);
2727
}
2828
public async lint(_document: TextDocument, _cancellation: CancellationToken): Promise<ILintMessage[]> {
2929
return [];
@@ -43,14 +43,14 @@ export class LinterManager implements ILinterManager {
4343
this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
4444
// Note that we use unit tests to ensure all the linters are here.
4545
this.linters = [
46-
new LinterInfo(Product.bandit, 'bandit', this.configService),
47-
new LinterInfo(Product.flake8, 'flake8', this.configService),
46+
new LinterInfo(Product.bandit, LinterId.Bandit, this.configService),
47+
new LinterInfo(Product.flake8, LinterId.Flake8, this.configService),
4848
new PylintLinterInfo(this.configService, this.workspaceService, ['.pylintrc', 'pylintrc']),
49-
new LinterInfo(Product.mypy, 'mypy', this.configService),
50-
new LinterInfo(Product.pycodestyle, 'pycodestyle', this.configService),
51-
new LinterInfo(Product.prospector, 'prospector', this.configService),
52-
new LinterInfo(Product.pydocstyle, 'pydocstyle', this.configService),
53-
new LinterInfo(Product.pylama, 'pylama', this.configService)
49+
new LinterInfo(Product.mypy, LinterId.MyPy, this.configService),
50+
new LinterInfo(Product.pycodestyle, LinterId.PyCodeStyle, this.configService),
51+
new LinterInfo(Product.prospector, LinterId.Prospector, this.configService),
52+
new LinterInfo(Product.pydocstyle, LinterId.PyDocStyle, this.configService),
53+
new LinterInfo(Product.pylama, LinterId.PyLama, this.configService)
5454
];
5555
}
5656

‎src/client/linters/types.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,16 @@ export interface IErrorHandler {
1111
handleError(error: Error, resource: vscode.Uri, execInfo: ExecutionInfo): Promise<boolean>;
1212
}
1313

14-
// tslint:disable-next-line:no-suspicious-comment
15-
// TODO: Use an enum for LinterID instead of a union of string literals.
16-
export type LinterId = 'flake8' | 'mypy' | 'pycodestyle' | 'prospector' | 'pydocstyle' | 'pylama' | 'pylint' | 'bandit';
14+
export enum LinterId {
15+
Flake8 = 'flake8',
16+
MyPy = 'mypy',
17+
PyCodeStyle = 'pycodestyle',
18+
Prospector = 'prospector',
19+
PyDocStyle = 'pydocstyle',
20+
PyLama = 'pylama',
21+
PyLint = 'pylint',
22+
Bandit = 'bandit'
23+
}
1724

1825
export interface ILinterInfo {
1926
readonly id: LinterId;

‎src/test/linters/lint.functional.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ const pythonFilesDir = path.join(workspaceDir, 'pythonFiles', 'linting');
4040
const fileToLint = path.join(pythonFilesDir, 'file.py');
4141

4242
const linterConfigDirs = new Map<LinterId, string>([
43-
['flake8', path.join(pythonFilesDir, 'flake8config')],
44-
['pycodestyle', path.join(pythonFilesDir, 'pycodestyleconfig')],
45-
['pydocstyle', path.join(pythonFilesDir, 'pydocstyleconfig27')],
46-
['pylint', path.join(pythonFilesDir, 'pylintconfig')]
43+
[LinterId.Flake8, path.join(pythonFilesDir, 'flake8config')],
44+
[LinterId.PyCodeStyle, path.join(pythonFilesDir, 'pycodestyleconfig')],
45+
[LinterId.PyDocStyle, path.join(pythonFilesDir, 'pydocstyleconfig27')],
46+
[LinterId.PyLint, path.join(pythonFilesDir, 'pylintconfig')]
4747
]);
4848
const linterConfigRCFiles = new Map<LinterId, string>([
49-
['pylint', '.pylintrc'],
50-
['pydocstyle', '.pydocstyle']
49+
[LinterId.PyLint, '.pylintrc'],
50+
[LinterId.PyDocStyle, '.pydocstyle']
5151
]);
5252

5353
const pylintMessagesToBeReturned: ILintMessage[] = [

‎src/test/linters/linter.availability.unit.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
import { Common, Linters } from '../../client/common/utils/localize';
2727
import { AvailableLinterActivator } from '../../client/linters/linterAvailability';
2828
import { LinterInfo } from '../../client/linters/linterInfo';
29-
import { IAvailableLinterActivator, ILinterInfo } from '../../client/linters/types';
29+
import { IAvailableLinterActivator, ILinterInfo, LinterId } from '../../client/linters/types';
3030

3131
// tslint:disable:max-func-body-length no-any
3232
suite('Linter Availability Provider tests', () => {
@@ -257,7 +257,7 @@ suite('Linter Availability Provider tests', () => {
257257
this.testIsEnabled = enabled;
258258
return Promise.resolve();
259259
}
260-
})(Product.pylint, 'pylint', configServiceMock.object, ['.pylintrc', 'pylintrc']);
260+
})(Product.pylint, LinterId.PyLint, configServiceMock.object, ['.pylintrc', 'pylintrc']);
261261

262262
const notificationPromptEnabled = TypeMoq.Mock.ofType<IPersistentState<boolean>>();
263263
factoryMock
@@ -856,6 +856,6 @@ function getDependenciesForAvailabilityTests(): [
856856
TypeMoq.Mock.ofType<IWorkspaceService>(),
857857
TypeMoq.Mock.ofType<IConfigurationService>(),
858858
TypeMoq.Mock.ofType<IPersistentStateFactory>(),
859-
new LinterInfo(Product.pylint, 'pylint', configServiceMock.object, ['.pylintrc', 'pylintrc'])
859+
new LinterInfo(Product.pylint, LinterId.PyLint, configServiceMock.object, ['.pylintrc', 'pylintrc'])
860860
];
861861
}

‎src/test/linters/mypy.unit.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import { expect } from 'chai';
99
import { parseLine } from '../../client/linters/baseLinter';
1010
import { REGEX } from '../../client/linters/mypy';
11-
import { ILintMessage } from '../../client/linters/types';
11+
import { ILintMessage, LinterId } from '../../client/linters/types';
1212

1313
// This following is a real-world example. See gh=2380.
1414
// tslint:disable-next-line:no-multiline-string
@@ -57,7 +57,7 @@ suite('Linting - MyPy', () => {
5757
]
5858
];
5959
for (const [line, expected] of tests) {
60-
const msg = parseLine(line, REGEX, 'mypy');
60+
const msg = parseLine(line, REGEX, LinterId.MyPy);
6161

6262
expect(msg).to.deep.equal(expected);
6363
}

0 commit comments

Comments
 (0)