Skip to content

Commit

Permalink
improve linting
Browse files Browse the repository at this point in the history
- use the TS version of `class-methods-use-this` so we can ignore override methods.
  • Loading branch information
irahopkinson committed Nov 14, 2024
1 parent a7b6d06 commit 3ea429c
Show file tree
Hide file tree
Showing 19 changed files with 29 additions and 55 deletions.
7 changes: 6 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ module.exports = {
// Rules in each section are generally in alphabetical order. However, several
// `@typescript-eslint` rules require disabling the equivalent ESLint rule. So in these cases
// each ESLint rule is turned off immediately above the corresponding `@typescript-eslint` rule.
'import/no-anonymous-default-export': ['error', { allowCallExpression: false }],
'class-methods-use-this': 'off',
'@typescript-eslint/class-methods-use-this': [
'error',
{ ignoreOverrideMethods: true, ignoreClassesThatImplementAnInterface: false },
],
'@typescript-eslint/explicit-member-accessibility': ['error', { accessibility: 'no-public' }],
'lines-between-class-members': 'off',
'@typescript-eslint/lines-between-class-members': [
Expand Down Expand Up @@ -75,6 +79,7 @@ module.exports = {
'no-useless-constructor': 'off',
'@typescript-eslint/no-useless-constructor': 'error',
'comma-dangle': ['error', 'always-multiline'],
'import/no-anonymous-default-export': ['error', { allowCallExpression: false }],
indent: 'off',
'jsx-a11y/label-has-associated-control': [
'error',
Expand Down
2 changes: 1 addition & 1 deletion extensions/src/hello-world/src/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class HelloCheck implements Check {
return true;
}

// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
getCheckDetails(): CheckDetails {
return checkDetails;
}
Expand Down
8 changes: 2 additions & 6 deletions extensions/src/hello-world/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ class HelloWorldProjectWebViewFactory extends WebViewFactory<
super(HELLO_WORLD_PROJECT_WEB_VIEW_TYPE);
}

// No need to use `this` and implementing an abstract method so can't be static
// eslint-disable-next-line class-methods-use-this
async getWebViewDefinition(
override async getWebViewDefinition(
savedWebView: SavedWebViewDefinition,
getWebViewOptions: HelloWorldProjectOptions,
): Promise<WebViewDefinition | undefined> {
Expand All @@ -130,9 +128,7 @@ class HelloWorldProjectWebViewFactory extends WebViewFactory<
};
}

// No need to use `this` and implementing an abstract method so can't be static
// eslint-disable-next-line class-methods-use-this
async createWebViewController(
override async createWebViewController(
webViewDefinition: WebViewDefinition,
webViewNonce: string,
): Promise<HelloWorldProjectWebViewController> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ class HelloWorldProjectDataProviderEngine
return newNumber;
}

// eslint-disable-next-line class-methods-use-this
// Required method since this is a data provider engine
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setNames(): Promise<
DataProviderUpdateInstructions<ProjectInterfaceDataTypes['helloWorld']>
> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const tooltipKey = '%paratextRegistration_webView_tooltip%';

export default class ParatextRegistrationWebViewProvider implements IWebViewProvider {
// needs to be a class method, not static method
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async getWebView(savedWebView: SavedWebViewDefinition): Promise<WebViewDefinition | undefined> {
if (savedWebView.webViewType !== paratextRegistrationWebViewType)
throw new Error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class CheckDataProviderEngine
lastRangesSet: CheckInputRange[] = [];

// Required method since this is a data provider engine
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setAvailableChecks(): Promise<DataProviderUpdateInstructions<CheckRunnerDataTypes>> {
throw new Error('setAvailableChecks disabled');
}
Expand All @@ -46,7 +46,7 @@ class CheckDataProviderEngine
}

// Required method since this is a data provider engine
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setCheckResults(): Promise<DataProviderUpdateInstructions<CheckRunnerDataTypes>> {
throw new Error('setCheckResults disabled');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class CheckRunnerEngine
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setAvailableChecks(): Promise<DataProviderUpdateInstructions<CheckRunnerDataTypes>> {
throw new Error('setAvailableChecks disabled - use enableCheck and disableCheck');
}
Expand Down Expand Up @@ -221,7 +221,7 @@ class CheckRunnerEngine
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setCheckResults(): Promise<DataProviderUpdateInstructions<CheckRunnerDataTypes>> {
throw new Error('setCheckResults disabled');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ class ScriptureExtenderProjectDataProviderEngine
return false;
}

// eslint-disable-next-line class-methods-use-this
// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setVerseUSJ(): Promise<DataProviderUpdateInstructions<USJVerseProjectInterfaceDataTypes>> {
throw new Error('Cannot call setVerseUSJ, use setChapterUSJ or setBookUSJ instead');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ScriptureExtenderProjectDataProviderEngineFactory
providedProjectInterfaces = SCRIPTURE_EXTENDER_PROJECT_INTERFACES;

// Implementing an interface method, so can't be static
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async createProjectDataProviderEngine(
projectId: string,
): Promise<IProjectDataProviderEngine<typeof SCRIPTURE_EXTENDER_PROJECT_INTERFACES>> {
Expand Down
2 changes: 1 addition & 1 deletion extensions/src/quick-verse/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class QuickVerseDataProviderEngine
* @returns False meaning do not update anything
*/
// Does nothing, so we don't need to use `this`
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setChapter() {
// We are not supporting setting chapters now, so don't update anything
return false;
Expand Down
6 changes: 3 additions & 3 deletions src/extension-host/services/localization.service-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class LocalizationDataProviderEngine
implements IDataProviderEngine<LocalizationDataDataTypes>
{
// This method legitimately does not need to call anything else in this class as of now
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async getLocalizedString({ localizeKey, locales = [] }: LocalizationSelector) {
const languages = locales.length > 0 ? [...locales] : await getDefaultLanguages();

Expand Down Expand Up @@ -300,13 +300,13 @@ class LocalizationDataProviderEngine
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setLocalizedString(): Promise<DataProviderUpdateInstructions<LocalizationDataDataTypes>> {
throw new Error('setLocalizedString disabled');
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setLocalizedStrings(): Promise<DataProviderUpdateInstructions<LocalizationDataDataTypes>> {
throw new Error('setLocalizedStrings disabled');
}
Expand Down
4 changes: 2 additions & 2 deletions src/extension-host/services/menu-data.service-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class MenuDataDataProviderEngine
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setMainMenu(): Promise<DataProviderUpdateInstructions<MenuDataDataTypes>> {
throw new Error('setMainMenu disabled');
}
Expand All @@ -68,7 +68,7 @@ class MenuDataDataProviderEngine
}

// Because this is a data provider, we have to provide this method even though it always throws
// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async setWebViewMenu(): Promise<DataProviderUpdateInstructions<MenuDataDataTypes>> {
throw new Error('setWebViewMenu disabled');
}
Expand Down
5 changes: 3 additions & 2 deletions src/extension-host/services/settings.service-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,10 @@ class SettingDataProviderEngine
);
}

// eslint-disable-next-line class-methods-use-this
/* eslint-disable @typescript-eslint/class-methods-use-this */
@dataProviders.decorators.ignore
async getLocalizedSettingsContributionInfo(): Promise<
/* eslint-enable */
LocalizedSettingsContributionInfo | undefined
> {
return settingsDocumentCombiner.getLocalizedSettingsContributionInfo();
Expand Down Expand Up @@ -174,7 +175,7 @@ class SettingDataProviderEngine
return true;
}

// eslint-disable-next-line class-methods-use-this
// eslint-disable-next-line @typescript-eslint/class-methods-use-this
async validateSetting<SettingName extends SettingNames>(
key: SettingName,
newValue: SettingTypes[SettingName],
Expand Down
2 changes: 1 addition & 1 deletion src/shared/models/data-provider-engine.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,6 @@ export abstract class DataProviderEngine<TDataTypes extends DataProviderDataType
// @ts-expect-error ts(6133) `updateInstructions` is not used in this method, but we don't care
// because we want inheriting classes to be able to get this method with Intellisense without
// an underscore that indicates to TypeScript that we aren't using the parameter
// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-unused-vars
// eslint-disable-next-line @typescript-eslint/class-methods-use-this, @typescript-eslint/no-unused-vars
notifyUpdate(updateInstructions?: DataProviderUpdateInstructions<TDataTypes>): void {}
}
10 changes: 0 additions & 10 deletions src/shared/utils/localized-strings-document-combiner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,10 @@ export default class LocalizedStringsDocumentCombiner extends DocumentCombiner {
) as T extends string ? LanguageStrings | undefined : LocalizedStringDataContribution;
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateBaseDocument(baseDocument: JsonDocumentLike): void {
performSchemaValidation(baseDocument, PLATFORM_NAMESPACE);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override transformBaseDocumentAfterValidation(
baseDocument: JsonDocumentLike,
): JsonDocumentLike {
Expand All @@ -105,14 +101,10 @@ export default class LocalizedStringsDocumentCombiner extends DocumentCombiner {
);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateContribution(_documentName: string, document: JsonDocumentLike): void {
performSchemaValidation(document, PLATFORM_NAMESPACE);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override transformContributionAfterValidation(
_documentName: string,
document: JsonDocumentLike,
Expand All @@ -124,8 +116,6 @@ export default class LocalizedStringsDocumentCombiner extends DocumentCombiner {
);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateOutput(output: JsonDocumentLike): void {
performSchemaValidation(output, 'output');
}
Expand Down
6 changes: 0 additions & 6 deletions src/shared/utils/menu-document-combiner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,6 @@ export default class MenuDocumentCombiner extends DocumentCombiner {
return retVal;
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateBaseDocument(baseDocument: JsonDocumentLike): void {
// The starting document has to validate against the output schema, too
performSchemaValidation(baseDocument, 'starting');
Expand Down Expand Up @@ -343,8 +341,6 @@ export default class MenuDocumentCombiner extends DocumentCombiner {
// TODO: Validate that extensions only add to objects that are marked as extensible
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateOutput(output: JsonDocumentLike): void {
performSchemaValidation(output, 'output');
// Once the schema has been validated, we know the type should match
Expand All @@ -371,8 +367,6 @@ export default class MenuDocumentCombiner extends DocumentCombiner {
}

// Combine the webview menu defaults for any web views that indicate that is desired
// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override transformFinalOutputBeforeValidation(
finalOutput: JsonDocumentLike,
): JsonDocumentLike {
Expand Down
2 changes: 0 additions & 2 deletions src/shared/utils/project-settings-document-combiner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ export default class ProjectSettingsDocumentCombiner extends SettingsDocumentCom
>;
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override performSchemaValidation(document: JsonDocumentLike, docType: string): void {
performSchemaValidation(document, docType);
}
Expand Down
10 changes: 0 additions & 10 deletions src/shared/utils/settings-document-combiner-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,10 @@ export default abstract class SettingsDocumentCombinerBase extends DocumentCombi
return this.localizedOutputPromise;
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateBaseDocument(baseDocument: JsonDocumentLike): void {
this.performSchemaValidation(baseDocument, PLATFORM_NAMESPACE);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override transformBaseDocumentAfterValidation(
baseDocument: JsonDocumentLike,
): JsonDocumentLike {
Expand All @@ -208,8 +204,6 @@ export default abstract class SettingsDocumentCombinerBase extends DocumentCombi
);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateContribution(documentName: string, document: JsonDocumentLike): void {
// Make sure it is a SettingsContribution
this.performSchemaValidation(document, documentName);
Expand Down Expand Up @@ -241,8 +235,6 @@ export default abstract class SettingsDocumentCombinerBase extends DocumentCombi
);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override transformContributionAfterValidation(
documentName: string,
document: JsonDocumentLike,
Expand All @@ -255,8 +247,6 @@ export default abstract class SettingsDocumentCombinerBase extends DocumentCombi
);
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override validateOutput(): void {
// We already validated input documents and built the output ourselves, so we don't have any more
// validating to do. Unless someday we want to double check we have a properly formatted
Expand Down
2 changes: 0 additions & 2 deletions src/shared/utils/settings-document-combiner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ export default class SettingsDocumentCombiner extends SettingsDocumentCombinerBa
return this.getLocalizedOutput();
}

// We don't need `this` on this override method
// eslint-disable-next-line class-methods-use-this
protected override performSchemaValidation(document: JsonDocumentLike, docType: string): void {
performSchemaValidation(document, docType);
}
Expand Down

0 comments on commit 3ea429c

Please sign in to comment.