Opened 23 months ago
Closed 10 months ago
#53837 closed enhancement (wontfix)
Move the basepath config out of phpcs.xml.dist
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch |
Focuses: | Cc: |
Description
The basepath
config option in phpcs.xml.dist is there to shorten the path names in PHPCS reports so they're relative to the project root directory rather than being absolute.
A few PHPCS tools I've used don't support a change to the base path and expect absolute paths in the report, and break without one. The most prominent example is the PHPCS extension for VS Code which has over a million installations. Unfortunately this particular extension hasn't seen any updates for three years, yet remains the number one PHPCS extension.
Clearly this is a problem with the third party tools but I've now seen several developers trying to contribute to WordPress core using VS Code and not being able to get PHPCS annotations working in their editor because the extension is expecting absolute paths in the report but receiving relative ones.
To solve the immediate problem we can move the basepath
config out of phpcs.xml.dist
and into the --basepath
flag in the lint
script in composer.json. This means external PHPCS tools will get the full paths but anything that uses composer lint
gets the relative paths.
Change History (6)
This ticket was mentioned in PR #1531 on WordPress/wordpress-develop by johnbillion.
23 months ago
#1
- Keywords has-patch added; needs-patch removed
johnbillion commented on PR #1531:
21 months ago
#2
it is a work-around for a select group of users using a specific tool, which makes things less convenient for the majority of users
My experience says this isn't the case, due to the way people most commonly use PHPCS. I work with various teams of experienced and inexperienced developers and almost none of them run PHPCS directly using ./vendor/bin/phpcs
or a global phpcs
, the majority of them interface with it as just another tool in their editor/IDE or via a project-level Composer or NPM script. VS Code has 14 million users (!) and the PHPCS extension over a million, so I'm sure this change will convenience many more users than those who are familiar with and use phpcs
directly. Plus it's an immediate fix for VS Code users who want to contribute to WordPress, which is the main thing.
I believe the Make Core handbook should probably be enhanced with a few additional pages, informing users about how to install and run PHPCS
Yeah agreed. Maybe one for the docs team and test team to collaborate on.
I do think it needs checking that this change does not impact the
cs2pr
integration in GH Actions
I don't _think_ it will impact it because the usage example given on https://github.com/staabm/annotate-pull-request-from-checkstyle uses a vanilla PHPCS configuration without any value in basepath
, but I'll check to make sure. Thanks.
21 months ago
#3
it is a work-around for a select group of users using a specific tool, which makes things less convenient for the majority of users
My experience says this isn't the case, due to the way people most commonly use PHPCS. I work with various teams of experienced and inexperienced developers and almost none of them run PHPCS directly using
./vendor/bin/phpcs
or a globalphpcs
, the majority of them interface with it as just another tool in their editor/IDE or via a project-level Composer or NPM script. VS Code has 14 million users (!) and the PHPCS extension over a million, so I'm sure this change will convenience many more users than those who are familiar with and usephpcs
directly. Plus it's an immediate fix for VS Code users who want to contribute to WordPress, which is the main thing.
But we're not talking about all VS Code users here, we're talking about WP Core contributors. And while I don't have access to stats, I know a wide variety of IDEs and code editors is used by those (PHPStorm, Sublime, Atom, Eclipse, GH Code spaces and yes, VS Code, to name a few) and this change is only for the users of VS Code and may or may not negatively impact users of other editors.
Have any tests been done to see if this change impacts users of other editors ?
johnbillion commented on PR #1531:
21 months ago
#4
The fix is to revert a non-default PHPCS setting to its default; what negative impact in other editors are you imagining?
21 months ago
#5
The fix is to revert a non-default PHPCS setting to its default; what negative impact in other editors are you imagining?
Not imagining anything, just wanted to ask the question as if we're making changes to accommodate one editor, it seemed sensible to make sure there is no impact on other editors.
Trac ticket: https://core.trac.wordpress.org/ticket/53837