Skip to content

Commit

Permalink
Stabilize Working Directory (#65)
Browse files Browse the repository at this point in the history
Changing the working directory based on whether or not an executable
was automatically found can lead to inconsistent behavior when using
relative paths with `phpCodeSniffer.standardCustom`. Initially this was
done so that the `"Default"` standard option would let PHPCS traverse
up to the workspace root. Since the `"Automatic"` standard type finds
the nearest PHPCS configuration, this is not necessary.
  • Loading branch information
ObliviousHarmony authored Apr 13, 2023
1 parent 8e9680a commit e59d4f0
Show file tree
Hide file tree
Showing 17 changed files with 224 additions and 144 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- `Automatic` option for `phpCodeSniffer.standard` that searches for a coding standard file
(`phpcs.xml`, `.phpcs.xml`, `phpcs.dist.xml`, `.phpcs.dist.xml`). The search begins in the
document's folder and traverses through parent directories until it reaches the workspace root.
document's folder and traverses through parent folders until it reaches the workspace root.
- `phpCodeSniffer.exec.linux`, `phpCodeSniffer.exec.osx`, and `phpCodeSniffer.exec.windows` options
for platform-specific executables.
- Support for execution on Windows without the use of WSL.

### Changed
- Even if `phpCodeSniffer.autoExecutable` is enabled, the working directory given to PHPCS should always be the workspace root.

### Deprecated
- `phpCodeSniffer.executable` has been deprecated in favor of platform-specific executable options.

### Fixed
- Gracefully handle errors caused by an invalid PHPCS executable setting.

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Allow PHPCS to decide what standard should apply to the document. It will either

#### `Automatic`

When selected, this option will cause the extension to search for an applicable coding standard file (`.phpcs.xml`, `phpcs.xml`, `.phpcs.xml.dist`, `phpcs.xml.dist`). The extension starts in the document's directory and traverses through parent directories until it reaches the workspace root. If the extension fails to find a file it will do nothing and output an error.
When selected, this option will cause the extension to search for an applicable coding standard file (`.phpcs.xml`, `phpcs.xml`, `.phpcs.xml.dist`, `phpcs.xml.dist`). The extension starts in the document's folder and traverses through parent directories until it reaches the workspace root. If the extension fails to find a file it will do nothing and output an error.

#### `Custom`

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"properties": {
"phpCodeSniffer.autoExecutable": {
"type": "boolean",
"markdownDescription": "Attempts to find `bin/phpcs` in the file directory and parent directories' vendor directory before falling back to the platform-specific executable.",
"markdownDescription": "Attempts to find `bin/phpcs` in the file folder and parent directories' vendor folder before falling back to the platform-specific executable.",
"default": false,
"scope": "resource"
},
Expand Down Expand Up @@ -107,7 +107,7 @@
"enumDescriptions": [
"Disables the PHP_CodeSniffer extension's linting.",
"Allows PHPCS to decide the coding standard.",
"Searches for a coding standard configuration file in the current document's directory and parent directories.",
"Searches for a coding standard configuration file in the current document's folder and parent directories.",
"Uses the PEAR coding standard.",
"Uses the MySource coding standard.",
"Uses the Squiz coding standard.",
Expand Down
7 changes: 6 additions & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { DiagnosticUpdater } from './services/diagnostic-updater';
import { DocumentFormatter } from './services/document-formatter';
import { Logger } from './services/logger';
import { LinterStatus } from './services/linter-status';
import { WorkspaceLocator } from './services/workspace-locator';

export function activate(context: ExtensionContext): void {
// We will store all of the diagnostics and code actions
Expand All @@ -29,10 +30,12 @@ export function activate(context: ExtensionContext): void {
// Create all of our dependencies.
const logger = new Logger(window);
const linterStatus = new LinterStatus(window);
const configuration = new Configuration(workspace);
const workspaceLocator = new WorkspaceLocator(workspace);
const configuration = new Configuration(workspace, workspaceLocator);
const workerPool = new WorkerPool(10);
const diagnosticUpdater = new DiagnosticUpdater(
logger,
workspaceLocator,
configuration,
workerPool,
linterStatus,
Expand All @@ -41,11 +44,13 @@ export function activate(context: ExtensionContext): void {
);
const codeActionEditResolver = new CodeActionEditResolver(
logger,
workspaceLocator,
configuration,
workerPool
);
const documentFormatter = new DocumentFormatter(
logger,
workspaceLocator,
configuration,
workerPool
);
Expand Down
4 changes: 2 additions & 2 deletions src/phpcs-report/__tests__/worker-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ describe('Worker/WorkerPool Integration', () => {
const promise = pool.waitForAvailable('test').then((worker) => {
const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -75,10 +75,10 @@ describe('Worker/WorkerPool Integration', () => {
const promise = pool.waitForAvailable('test').then((worker) => {
const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand Down
14 changes: 7 additions & 7 deletions src/phpcs-report/__tests__/worker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ describe('Worker', () => {

const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -68,10 +68,10 @@ describe('Worker', () => {

const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -92,10 +92,10 @@ describe('Worker', () => {

const request: Request<ReportType.CodeAction> = {
type: ReportType.CodeAction,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand Down Expand Up @@ -132,10 +132,10 @@ describe('Worker', () => {

const request: Request<ReportType.Format> = {
type: ReportType.Format,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -154,10 +154,10 @@ describe('Worker', () => {

const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -180,10 +180,10 @@ describe('Worker', () => {

const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
executable: phpcsPath,
standard: 'PSR12',
},
Expand All @@ -201,10 +201,10 @@ describe('Worker', () => {

const request: Request<ReportType.Diagnostic> = {
type: ReportType.Diagnostic,
workingDirectory: __dirname,
documentPath: 'Test.php',
documentContent: '<?php class Test {}',
options: {
workingDirectory: __dirname,
// Since we use custom reports, adding `-s` for sources won't break anything.
executable: phpcsPath + ' -s',
standard: 'PSR12',
Expand Down
6 changes: 5 additions & 1 deletion src/phpcs-report/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { ReportType } from './response';
* The request options for PHPCS.
*/
export interface RequestOptions {
workingDirectory: string;
executable: string;
standard: string | null;
}
Expand Down Expand Up @@ -50,6 +49,11 @@ export interface Request<T extends ReportType> {
*/
type: T;

/**
* The working directory for the workspace that the report is being generated for.
*/
workingDirectory: string;

/**
* The filesystem path for the document that the report is being generated for.
*/
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 @@ -205,6 +205,7 @@ export class Worker {

// Prepare the options for the PHPCS process.
const processOptions: SpawnOptionsWithoutStdio = {
cwd: request.workingDirectory,
env: {
...process.env,
// Pass the request data using environment vars.
Expand All @@ -216,11 +217,6 @@ export class Worker {
windowsHide: true,
};

// Give the working directory when requested.
if (request.options.workingDirectory) {
processOptions.cwd = request.options.workingDirectory;
}

// Make sure PHPCS knows to read from STDIN.
processArguments.push('-');

Expand Down
Loading

0 comments on commit e59d4f0

Please sign in to comment.