From 8145cf31a08e29961d4739fe983a572a6223a17a Mon Sep 17 00:00:00 2001 From: Christopher Allford <6451942+ObliviousHarmony@users.noreply.github.com> Date: Mon, 29 Jan 2024 12:04:38 -0800 Subject: [PATCH] Revert "Support Overriding PHPCS Integration Path (#89)" This reverts commit 84b783a6be5fb0d39a77c2c21ed2890b0e76b605. --- .../__tests__/worker-integration.test.ts | 10 ++--- src/phpcs-report/__tests__/worker.spec.ts | 15 ++----- src/phpcs-report/request.ts | 1 - src/phpcs-report/worker.ts | 7 +++- src/services/__tests__/configuration.spec.ts | 4 -- .../__tests__/diagnostic-updater.spec.ts | 14 ++----- src/services/code-action-edit-resolver.ts | 1 - src/services/configuration.ts | 41 ------------------- src/services/diagnostic-updater.ts | 12 +++--- src/services/document-formatter.ts | 1 - 10 files changed, 23 insertions(+), 83 deletions(-) diff --git a/src/phpcs-report/__tests__/worker-integration.test.ts b/src/phpcs-report/__tests__/worker-integration.test.ts index 537d26b..1587786 100644 --- a/src/phpcs-report/__tests__/worker-integration.test.ts +++ b/src/phpcs-report/__tests__/worker-integration.test.ts @@ -8,18 +8,18 @@ import { MockCancellationToken } from '../../__mocks__/vscode'; // We need to mock the report files because Webpack is being used to bundle them. describe('Worker/WorkerPool Integration', () => { - let phpcsIntegrationPath: string; let phpcsPath: string; beforeAll(() => { - phpcsIntegrationPath = resolvePath( + // Make sure the test knows where the real assets are located. + process.env.ASSETS_PATH = resolvePath( __dirname, '..', '..', '..', - 'assets', - 'phpcs-integration' + 'assets' ); + phpcsPath = resolvePath( __dirname, '..', @@ -53,7 +53,6 @@ describe('Worker/WorkerPool Integration', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; @@ -82,7 +81,6 @@ describe('Worker/WorkerPool Integration', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; diff --git a/src/phpcs-report/__tests__/worker.spec.ts b/src/phpcs-report/__tests__/worker.spec.ts index d51607b..312f70e 100644 --- a/src/phpcs-report/__tests__/worker.spec.ts +++ b/src/phpcs-report/__tests__/worker.spec.ts @@ -8,18 +8,18 @@ import { Worker } from '../worker'; // We need to mock the report files because Webpack is being used to bundle them. describe('Worker', () => { - let phpcsIntegrationPath: string; let phpcsPath: string; beforeAll(() => { - phpcsIntegrationPath = resolvePath( + // Make sure the test knows where the real assets are located. + process.env.ASSETS_PATH = resolvePath( __dirname, '..', '..', '..', - 'assets', - 'phpcs-integration' + 'assets' ); + phpcsPath = resolvePath( __dirname, '..', @@ -52,7 +52,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; @@ -75,7 +74,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; @@ -100,7 +98,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: { code: 'PSR12.Files.OpenTag.NotAlone', @@ -141,7 +138,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: {}, }; @@ -164,7 +160,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; @@ -191,7 +186,6 @@ describe('Worker', () => { options: { executable: phpcsPath, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; @@ -214,7 +208,6 @@ describe('Worker', () => { // Since we use custom reports, adding `-s` for sources won't break anything. executable: phpcsPath + ' -s', standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, data: null, }; diff --git a/src/phpcs-report/request.ts b/src/phpcs-report/request.ts index dd06a9a..dd0fa2c 100644 --- a/src/phpcs-report/request.ts +++ b/src/phpcs-report/request.ts @@ -6,7 +6,6 @@ import { ReportType } from './response'; export interface RequestOptions { executable: string; standard: string | null; - phpcsIntegrationPath: string; } /** diff --git a/src/phpcs-report/worker.ts b/src/phpcs-report/worker.ts index 77eb22c..4d20f2e 100644 --- a/src/phpcs-report/worker.ts +++ b/src/phpcs-report/worker.ts @@ -171,12 +171,17 @@ export class Worker { resolve: (response: Response) => void, reject: (e?: unknown) => void ): ChildProcess { + // Figure out the path to the PHPCS integration. + const assetPath = + process.env.ASSETS_PATH || resolvePath(__dirname, 'assets'); + // Prepare the arguments for our PHPCS process. const processArguments = [ '-q', // Make sure custom configs never break our output. '--report=' + resolvePath( - request.options.phpcsIntegrationPath, + assetPath, + 'phpcs-integration', 'VSCodeIntegration.php' ), // We want to reserve error exit codes for actual errors in the PHPCS execution since errors/warnings are expected. diff --git a/src/services/__tests__/configuration.spec.ts b/src/services/__tests__/configuration.spec.ts index 39e5b56..70062f5 100644 --- a/src/services/__tests__/configuration.spec.ts +++ b/src/services/__tests__/configuration.spec.ts @@ -13,7 +13,6 @@ import { TextEncoder } from 'util'; import { Configuration, LintAction, - SpecialOptions, SpecialStandardOptions, } from '../configuration'; import { WorkspaceLocator } from '../workspace-locator'; @@ -30,7 +29,6 @@ type ConfigurationType = { lintAction: LintAction; standard: SpecialStandardOptions | string; standardCustom: string; - specialOptions: SpecialOptions; // Deprecated Options executable: string | null; @@ -61,8 +59,6 @@ const getDefaultConfiguration = (overrides?: Partial) => { return SpecialStandardOptions.Disabled; case 'standardCustom': return ''; - case 'specialOptions': - return {}; // Deprecated Settings case 'executable': diff --git a/src/services/__tests__/diagnostic-updater.spec.ts b/src/services/__tests__/diagnostic-updater.spec.ts index 8f31f13..5f20045 100644 --- a/src/services/__tests__/diagnostic-updater.spec.ts +++ b/src/services/__tests__/diagnostic-updater.spec.ts @@ -44,7 +44,6 @@ jest.mock('../../types', () => { }); describe('DiagnosticUpdater', () => { - let phpcsIntegrationPath: string; let mockLogger: Logger; let mockWorkspaceLocator: WorkspaceLocator; let mockConfiguration: Configuration; @@ -55,15 +54,15 @@ describe('DiagnosticUpdater', () => { let diagnosticUpdater: DiagnosticUpdater; beforeEach(() => { - phpcsIntegrationPath = resolvePath( + // Make sure the test knows where the real assets are located. + process.env.ASSETS_PATH = resolvePath( __dirname, '..', '..', '..', - '..', - 'assets', - 'phpcs-integration' + 'assets' ); + mockLogger = new Logger(window); mockWorkspaceLocator = new WorkspaceLocator(workspace); mockConfiguration = new Configuration(workspace, mockWorkspaceLocator); @@ -105,7 +104,6 @@ describe('DiagnosticUpdater', () => { exclude: [], lintAction: LintAction.Change, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }); jest.mocked(mockWorker).execute.mockImplementation((request) => { expect(request).toMatchObject({ @@ -114,7 +112,6 @@ describe('DiagnosticUpdater', () => { options: { executable: 'phpcs-test', standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, }); @@ -169,7 +166,6 @@ describe('DiagnosticUpdater', () => { exclude: [], lintAction: LintAction.Change, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }); jest.mocked(mockWorker).execute.mockImplementation((request) => { expect(request).toMatchObject({ @@ -178,7 +174,6 @@ describe('DiagnosticUpdater', () => { options: { executable: 'phpcs-test', standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }, }); @@ -203,7 +198,6 @@ describe('DiagnosticUpdater', () => { exclude: [], lintAction: LintAction.Save, standard: 'PSR12', - phpcsIntegrationPath: phpcsIntegrationPath, }); return diagnosticUpdater.update(document, LintAction.Change); diff --git a/src/services/code-action-edit-resolver.ts b/src/services/code-action-edit-resolver.ts index a8c52d4..f1d5753 100644 --- a/src/services/code-action-edit-resolver.ts +++ b/src/services/code-action-edit-resolver.ts @@ -73,7 +73,6 @@ export class CodeActionEditResolver extends WorkerService { options: { executable: config.executable, standard: config.standard, - phpcsIntegrationPath: config.phpcsIntegrationPath, }, data: { code: diagnostic.code as string, diff --git a/src/services/configuration.ts b/src/services/configuration.ts index b8c8b22..5c4d689 100644 --- a/src/services/configuration.ts +++ b/src/services/configuration.ts @@ -1,4 +1,3 @@ -import { resolve as resolvePath } from 'path'; import { TextDecoder } from 'util'; import { Minimatch } from 'minimatch'; import { @@ -55,16 +54,6 @@ export enum LintAction { Force = 'Force', } -/** - * An interface describing the narrow use-case options in the `phpCodeSniffer.specialOptions` configuration. - */ -export interface SpecialOptions { - /** - * An override for the path to the directory containing the extension's PHPCS integration files. - */ - phpcsIntegrationPathOverride?: string; -} - /** * An interface describing the configuration parameters we can read from the filesystem. */ @@ -81,7 +70,6 @@ interface ParamsFromConfiguration { exclude: RegExp[]; lintAction: LintAction; standard: string | null; - phpcsIntegrationPath: string; } /** @@ -107,11 +95,6 @@ export interface DocumentConfiguration { * The standard we should use when executing reports. */ standard: string | null; - - /** - * The path to the PHPCS integration files. - */ - phpcsIntegrationPath: string; } /** @@ -234,7 +217,6 @@ export class Configuration { exclude: fromConfig.exclude, lintAction: fromConfig.lintAction, standard: fromConfig.standard, - phpcsIntegrationPath: fromConfig.phpcsIntegrationPath, }; this.cache.set(document.uri, config); @@ -368,35 +350,12 @@ export class Configuration { cancellationToken ); - const specialOptions = config.get('specialOptions'); - if (specialOptions === undefined) { - throw new ConfigurationError( - 'specialOptions', - 'Value must be an object.' - ); - } - - // Use the default integration path unless overridden. - let phpcsIntegrationPath: string; - if (specialOptions.phpcsIntegrationPathOverride) { - phpcsIntegrationPath = specialOptions.phpcsIntegrationPathOverride; - } else { - // Keep in mind that after bundling the integration files will be in a different location - // than they are in development and we need to resolve the correct path. - phpcsIntegrationPath = resolvePath( - __dirname, - 'assets', - 'phpcs-integration' - ); - } - return { autoExecutable, executable, exclude, lintAction, standard, - phpcsIntegrationPath, }; } diff --git a/src/services/diagnostic-updater.ts b/src/services/diagnostic-updater.ts index d345c8d..17cefeb 100644 --- a/src/services/diagnostic-updater.ts +++ b/src/services/diagnostic-updater.ts @@ -110,12 +110,12 @@ export class DiagnosticUpdater extends WorkerService { this.configuration .get(document, cancellationToken) - .then((config) => { + .then((configuration) => { // Allow users to decide when the diagnostics are updated. switch (lintAction) { case LintAction.Change: // Allow users to restrict updates to explicit save actions. - if (config.lintAction === LintAction.Save) { + if (configuration.lintAction === LintAction.Save) { throw new UpdatePreventedError(); } @@ -123,7 +123,7 @@ export class DiagnosticUpdater extends WorkerService { case LintAction.Save: // When linting on change, we will always have the latest diagnostics, and don't need to update. - if (config.lintAction === LintAction.Change) { + if (configuration.lintAction === LintAction.Change) { throw new UpdatePreventedError(); } break; @@ -147,10 +147,8 @@ export class DiagnosticUpdater extends WorkerService { documentPath: document.uri.fsPath, documentContent: document.getText(), options: { - executable: config.executable, - standard: config.standard, - phpcsIntegrationPath: - config.phpcsIntegrationPath, + executable: configuration.executable, + standard: configuration.standard, }, data: null, }; diff --git a/src/services/document-formatter.ts b/src/services/document-formatter.ts index fef9514..9249233 100644 --- a/src/services/document-formatter.ts +++ b/src/services/document-formatter.ts @@ -75,7 +75,6 @@ export class DocumentFormatter extends WorkerService { options: { executable: config.executable, standard: config.standard, - phpcsIntegrationPath: config.phpcsIntegrationPath, }, data: data, };