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

Support Overriding PHPCS Integration Path #89

Merged
merged 2 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- `phpCodeSniffer.specialOptions` option to allow for supporting narrow use-cases without
needing to add options that most users will not need.
- `phpCodeSniffer.specialOptions.phpcsIntegrationPathOverride` option that allows for
overriding the path to the directory containing the extension's PHPCS integration.

## [2.2.0] - 2023-11-07
### Fixed
Expand Down
13 changes: 13 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@
"default": null,
"scope": "machine-overridable"
},
"phpCodeSniffer.specialOptions": {
"type": "object",
"description": "An object of special options for the extension that serve more narrow use-cases.",
"default": {},
"properties": {
"phpcsIntegrationPathOverride": {
"type": "string",
"description": "Overrides the path to the directory that contains the extension's PHPCS integration."
}
},
"additionalProperties": false,
"scope": "resource"
},
"phpCodeSniffer.ignorePatterns": {
"type": "array",
"description": "An array of regular expressions for paths that should be ignored by the extension.",
Expand Down
10 changes: 6 additions & 4 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(() => {
// Make sure the test knows where the real assets are located.
process.env.ASSETS_PATH = resolvePath(
phpcsIntegrationPath = resolvePath(
__dirname,
'..',
'..',
'..',
'assets'
'assets',
'phpcs-integration'
);

phpcsPath = resolvePath(
__dirname,
'..',
Expand Down Expand Up @@ -53,6 +53,7 @@ describe('Worker/WorkerPool Integration', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand Down Expand Up @@ -81,6 +82,7 @@ describe('Worker/WorkerPool Integration', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand Down
15 changes: 11 additions & 4 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(() => {
// Make sure the test knows where the real assets are located.
process.env.ASSETS_PATH = resolvePath(
phpcsIntegrationPath = resolvePath(
__dirname,
'..',
'..',
'..',
'assets'
'assets',
'phpcs-integration'
);

phpcsPath = resolvePath(
__dirname,
'..',
Expand Down Expand Up @@ -52,6 +52,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -74,6 +75,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -98,6 +100,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: {
code: 'PSR12.Files.OpenTag.NotAlone',
Expand Down Expand Up @@ -138,6 +141,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: {},
};
Expand All @@ -160,6 +164,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -186,6 +191,7 @@ describe('Worker', () => {
options: {
executable: phpcsPath,
standard: 'PSR12',
phpcsIntegrationPath: phpcsIntegrationPath,
},
data: null,
};
Expand All @@ -208,6 +214,7 @@ 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: 1 addition & 0 deletions src/phpcs-report/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ReportType } from './response';
export interface RequestOptions {
executable: string;
standard: string | null;
phpcsIntegrationPath: string;
}

/**
Expand Down
6 changes: 1 addition & 5 deletions src/phpcs-report/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,11 @@ 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(assetPath, 'phpcs-integration', 'VSCode.php'),
resolvePath(request.options.phpcsIntegrationPath, 'VSCode.php'),
// We want to reserve error exit codes for actual errors in the PHPCS execution since errors/warnings are expected.
'--runtime-set',
'ignore_warnings_on_exit',
Expand Down
4 changes: 4 additions & 0 deletions src/services/__tests__/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { TextEncoder } from 'util';
import {
Configuration,
LintAction,
SpecialOptions,
SpecialStandardOptions,
} from '../configuration';
import { WorkspaceLocator } from '../workspace-locator';
Expand All @@ -29,6 +30,7 @@ type ConfigurationType = {
lintAction: LintAction;
standard: SpecialStandardOptions | string;
standardCustom: string;
specialOptions: SpecialOptions;

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

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

describe('DiagnosticUpdater', () => {
let phpcsIntegrationPath: string;
let mockLogger: Logger;
let mockWorkspaceLocator: WorkspaceLocator;
let mockConfiguration: Configuration;
Expand All @@ -54,15 +55,15 @@ describe('DiagnosticUpdater', () => {
let diagnosticUpdater: DiagnosticUpdater;

beforeEach(() => {
// Make sure the test knows where the real assets are located.
process.env.ASSETS_PATH = resolvePath(
phpcsIntegrationPath = resolvePath(
__dirname,
'..',
'..',
'..',
'assets'
'..',
'assets',
'phpcs-integration'
);

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

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

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

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

/**
Expand All @@ -88,6 +100,11 @@ 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 @@ -210,6 +227,7 @@ 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 @@ -343,12 +361,35 @@ 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: 7 additions & 5 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((configuration) => {
.then((config) => {
// Allow users to decide when the diagnostics are updated.
switch (lintAction) {
case LintAction.Change:
// Allow users to restrict updates to explicit save actions.
if (configuration.lintAction === LintAction.Save) {
if (config.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 (configuration.lintAction === LintAction.Change) {
if (config.lintAction === LintAction.Change) {
throw new UpdatePreventedError();
}
break;
Expand All @@ -147,8 +147,10 @@ export class DiagnosticUpdater extends WorkerService {
documentPath: document.uri.fsPath,
documentContent: document.getText(),
options: {
executable: configuration.executable,
standard: configuration.standard,
executable: config.executable,
standard: config.standard,
phpcsIntegrationPath:
config.phpcsIntegrationPath,
},
data: null,
};
Expand Down
Loading
Loading