Skip to content

Commit

Permalink
Revert "Support Overriding PHPCS Integration Path (#89)"
Browse files Browse the repository at this point in the history
This reverts commit 84b783a.
  • Loading branch information
ObliviousHarmony committed Jan 29, 2024
1 parent d630d50 commit 8145cf3
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 83 deletions.
10 changes: 4 additions & 6 deletions src/phpcs-report/__tests__/worker-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
'..',
Expand Down Expand Up @@ -53,7 +53,6 @@ describe('Worker/WorkerPool Integration', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand Down Expand Up @@ -82,7 +81,6 @@ describe('Worker/WorkerPool Integration', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand Down
15 changes: 4 additions & 11 deletions src/phpcs-report/__tests__/worker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
'..',
Expand Down Expand Up @@ -52,7 +52,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -75,7 +74,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -100,7 +98,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: {
code: 'PSR12.Files.OpenTag.NotAlone',
Expand Down Expand Up @@ -141,7 +138,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: {},
};
Expand All @@ -164,7 +160,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -191,7 +186,6 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -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,
};
Expand Down
1 change: 0 additions & 1 deletion src/phpcs-report/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { ReportType } from './response';
export interface RequestOptions {
executable: string;
standard: string | null;
phpcsIntegrationPath: string;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/phpcs-report/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,17 @@ export class Worker {
resolve: (response: Response<T>) => 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.
Expand Down
4 changes: 0 additions & 4 deletions src/services/__tests__/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { TextEncoder } from 'util';
import {
Configuration,
LintAction,
SpecialOptions,
SpecialStandardOptions,
} from '../configuration';
import { WorkspaceLocator } from '../workspace-locator';
Expand All @@ -30,7 +29,6 @@ type ConfigurationType = {
lintAction: LintAction;
standard: SpecialStandardOptions | string;
standardCustom: string;
specialOptions: SpecialOptions;

// Deprecated Options
executable: string | null;
Expand Down Expand Up @@ -61,8 +59,6 @@ const getDefaultConfiguration = (overrides?: Partial<ConfigurationType>) => {
return SpecialStandardOptions.Disabled;
case 'standardCustom':
return '';
case 'specialOptions':
return {};

// Deprecated Settings
case 'executable':
Expand Down
14 changes: 4 additions & 10 deletions src/services/__tests__/diagnostic-updater.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ jest.mock('../../types', () => {
});

describe('DiagnosticUpdater', () => {
let phpcsIntegrationPath: string;
let mockLogger: Logger;
let mockWorkspaceLocator: WorkspaceLocator;
let mockConfiguration: Configuration;
Expand All @@ -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);
Expand Down Expand Up @@ -105,7 +104,6 @@ describe('DiagnosticUpdater', () => {
exclude: [],
lintAction: LintAction.Change,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
});
jest.mocked(mockWorker).execute.mockImplementation((request) => {
expect(request).toMatchObject({
Expand All @@ -114,7 +112,6 @@ describe('DiagnosticUpdater', () => {
options: {
executable: 'phpcs-test',
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
});

Expand Down Expand Up @@ -169,7 +166,6 @@ describe('DiagnosticUpdater', () => {
exclude: [],
lintAction: LintAction.Change,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
});
jest.mocked(mockWorker).execute.mockImplementation((request) => {
expect(request).toMatchObject({
Expand All @@ -178,7 +174,6 @@ describe('DiagnosticUpdater', () => {
options: {
executable: 'phpcs-test',
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
});

Expand All @@ -203,7 +198,6 @@ describe('DiagnosticUpdater', () => {
exclude: [],
lintAction: LintAction.Save,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
});

return diagnosticUpdater.update(document, LintAction.Change);
Expand Down
1 change: 0 additions & 1 deletion src/services/code-action-edit-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
41 changes: 0 additions & 41 deletions src/services/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { resolve as resolvePath } from 'path';
import { TextDecoder } from 'util';
import { Minimatch } from 'minimatch';
import {
Expand Down Expand Up @@ -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.
*/
Expand All @@ -81,7 +70,6 @@ interface ParamsFromConfiguration {
exclude: RegExp[];
lintAction: LintAction;
standard: string | null;
phpcsIntegrationPath: string;
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -368,35 +350,12 @@ export class Configuration {
cancellationToken
);

const specialOptions = config.get<SpecialOptions>('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,
};
}

Expand Down
12 changes: 5 additions & 7 deletions src/services/diagnostic-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,20 @@ 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();
}

break;

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;
Expand All @@ -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,
};
Expand Down
1 change: 0 additions & 1 deletion src/services/document-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ export class DocumentFormatter extends WorkerService {
options: {
executable: config.executable,
standard: config.standard,
phpcsIntegrationPath: config.phpcsIntegrationPath,
},
data: data,
};
Expand Down

0 comments on commit 8145cf3

Please sign in to comment.