Problem/Motivation

I am using this plugin to register several phpcs sniffs from different repos into my installed_paths with phpcs. I am also using Travis CI's caching directives in combination with phpcs' caching directives in order to cache the results of scans between runs for files that have not been modified. However, it appears that this plugin doesn't register installed_paths in a consistent way between CI runs, which invalidates the phpcs cache, thus negating the benefits of caching between runs.

Expected behaviour

Runs of this plugin between CI jobs should produce consistent output for the value of installed_paths (specifically, the order of the paths).

Actual behaviour

The order of the paths in installed_paths is not always the same between CI runs. This is likely due to a race condition of when certain packages finish installing.

Steps to reproduce

Use a standard in composer.json that references multiple standards, or reference multiple standards in composer.json (see example below). Create a Travis CI job to run phpcs and echo the value of installed_paths. Note that it will not always be in the same order between runs.

Example for run 1:

"installed_paths": "../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs,../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs",

Example for run 2:

"installed_paths": "../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs,../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs",

Contents of composer.json:

{
  "require-dev": {
    "alleyinteractive/alley-coding-standards": "^0.2.0"
},

Contents of composer.json in alley-coding-standards:

{
  "name": "alleyinteractive/alley-coding-standards",
  "description": "PHPCS sniffs for Alley Interactive",
  "type": "phpcodesniffer-standard",
  "license": "GPL-2.0-or-later",
  "require": {
    "squizlabs/php_codesniffer": "^3.5.0",
    "dealerdirect/phpcodesniffer-composer-installer": "*",
    "wp-coding-standards/wpcs": "^2.3.0",
    "automattic/vipwpcs": "^2.1.0",
    "phpcompatibility/phpcompatibility-wp": "*"
  },
  "require-dev": {}
}

Proposed changes

Make the installed_paths value consistent between runs by alphabetizing the values. This will allow repos that use this plugin to leverage Travis CI and phpcs caching for unmodified files by ensuring that the configuration values in the cache file, specifically the value for installed_paths, is consistent between runs if nothing changed.

Environment

| Question | Answer | ------------------------| ------- | OS | Linux, Ubuntu Bionic, running on Travis CI | PHP version | 7.3.14 | Composer version | 2.0.8 | PHP_CodeSniffer version | 3.5.8 | Dealerdirect PHPCS plugin version | 0.7.0 | Install type | project local

Output of vendor/bin/phpcs --config-show:

Using config file: /home/travis/build/alleyinteractive/brookings/vendor/squizlabs/php_codesniffer/CodeSniffer.conf

Contents of CodeSniffer.conf:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '../../alleyinteractive/alley-coding-standards,../../wp-coding-standards/wpcs,../../sirbrillig/phpcs-variable-analysis,../../automattic/vipwpcs',
)
?>

Tested against master branch?

• [x] I have verified the issue still exists in the master branch.

I have not verified this against the master branch, but looking at the diff between 0.7.0 and the current HEAD, there shouldn't be anything in that diff that would affect this issue.

Proposed Changes

Minimal changes to updated the plugin to support Composer 2.0 which is expected late May/beginning of June.

Tested by @danepowell and myself (Windows 7). Further testing would be very welcome!

Fixes #108

@mjrider @Potherca Should this be added to the 0.6.3 or 0.7 milestone ?

Related Issues

Changes are based on guidance found in:

• https://github.com/Dealerdirect/phpcodesniffer-composer-installer/issues/108
• https://github.com/composer/composer/issues/8726
• https://github.com/composer/composer/blob/master/UPGRADE-2.0.md#for-integrators-and-plugin-authors

Proposed Changes

Use multi-line conditions to reduce line-length.

Note: this doesn't solve all the "warnings" and AFAICS, the other warnings can't really be solved anyhow as it would necessitate breaking up command to multiple lines which would break the command.

Related Issues

#123

The release of Composer 2.0 is imminent, and it requires updates for composer plugins such as this package. Until this package is explicitly made compatible with Composer 2, it cannot be installed with Composer 2 (even as a dependency of another package).

This package needs to be updated not only to ensure compatibility with Composer 2, but to unblock downstream projects that rely on it and want to support composer 2.

Hopefully you received an email from the Composer maintainer about this, but if not let me know and I can forward it to you or put you in contact. Cheers.

Problem/Motivation

Possibly related to #32.

With the following in my composer:

"dealerdirect/phpcodesniffer-composer-installer": "^0.4",
"wimg/php-compatibility": "dev-feature/prevent-conflicts-with-other-standards",
"wp-coding-standards/wpcs": "dev-develop"

the installed_paths phpcs config value is wrong.

Composer resolves this as using PHPCS 3.0.2, and the relative branches of WPCS and PHPCompatibility, both of which support PHPCS 3.*.

Expected behaviour

I expect the paths to be correct. With 0.3 of this plugin. I get:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '/Users/gary/Local Sites/incipio/app/public/wp-content/themes/incipio/vendor/wp-coding-standards/wpcs,/Users/gary/Local Sites/incipio/app/public/wp-content/themes/incipio/vendor/wimg/php-compatibility',
)
?>

phpcs -i shows all of the standards correctly available, and phpcs runs correctly against the phpcs.xml.dist in my project.

Actual behaviour

With 0.4 of the plugin, I get:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '../../wp-coding-standards/wpcs/WordPress/,../../wp-coding-standards/wpcs/WordPress-Core/,../../wp-coding-standards/wpcs/WordPress-Docs/,../../wp-coding-standards/wpcs/WordPress-Extra/,../../wp-coding-standards/wpcs/WordPress-VIP/,../../wimg/php-compatibility/PHPCompatibility/',
)
?>

phpcs -i still does show the right standards are available, but, phpcs does not run correctly. Depending on the first called ruleset in phpcs.xml.dist, I get an error the form:

ERROR: Referenced sniff "WordPress" does not exist

This also happens if phpcs . --standard=WordPress is called.

Interestingly, calling phpcs . --standard=WordPress-Core does appear to run.

I'm not sure where the issue lies exactly, but @jrfnl says:

PHPCS 3.x is supposed to support standards as a top-level folder (as in how PHPCompatibility used to be layed out and how DealerDirect registered the paths now), but that looks to be buggy based on your findings.

I can run some tests with that and will report my finding to PHPCS, but basically as long as the installed_paths are registered in the PHPCS 2.x way (which is also still the recommendation for PHPCS 3.x), things should be fine.

Introduction

This PR creates an initial test framework to allow for creating and running integration/end-to-end tests for this plugin.

The framework as I have set it up:

• Allows for running tests both as a "project local" as well as a Composer global install without screwing up the Composer global environment of the user running the tests. Tests are run in the system temp directory and the tests clean up after themselves.
• Will ensure that the tests will run with the plugin as it is in the branch being tested.
• As the tests start new PHP processes, the framework ensures that the PHP version used to initiate the test run will be used in those processes. This prevents a test run on a dev machine using the system default PHP version, which would skew the test results.
• Similarly, the framework can be told to use a specific Composer phar file to run the tests with, rather than the system default version.
• The framework also allows for setting up "fake" PHPCS standards for use in the tests, which will make writing the tests simpler as we don't need to keep track of changes in external standards and don't need to take those into account in the tests.
• Tests can be run against multiple PHPCS versions using data providers and a helper class is included to get a specific or random) selection of versions to use compatible with the PHP version against which the tests are being run.
• And to aid in debugging failing tests, it includes a TestListener which will spit out all the CLI output seen for a particular test when the test fails.

The tests included in this PR are largely the tests which were already previously being run via the GH Actions workflow, though they are now encapsuled within a PHPUnit based framework and will be run via GH Actions against all supported PHP version, multiple versions of Composer and on multiple OSes.

A lot more tests are still needed, but those should be relatively straight-forward to add once this initial framework is in place and I have already started preparing some follow-up PRs to be pulled once this PR has been merged.

⚠️ This PR depends on PRs #147 and #152 both being accepted and merged.

👉🏻 As this is a huge PR, I have split it up into atomic commits telling the "story" and I would strongly recommend reviewing this PR by looking at each commit separately.

Proposed Changes

Tests: create initial setup

• Require the PHPUnit Polyfills as a basis for the tests. The PHPUnit Polyfills will ensure a compatible PHPUnit version will be available and will allow for writing tests for the latest version of PHPUnit (9.x), while still being able to run the tests on PHPUnit 4-8.
• Include a basic test bootstrap file which will allow for running the tests both via a Composer installed version of PHPUnit as well as via a PHPUnit PHAR file. Note: the bootstrap file will try to determine where the composer.phar file is located. For running the tests locally and/or overruling the default version of Composer, a COMPOSER_PHAR environment variable can be set in a local phpunit.xml overload file.
• Include information about the test setup in the CONTRIBUTING documentation.
• Includes adding new jobs to the "Quick test" and "Integration test" GH Actions workflows. Once the existing tests in the old workflows have been converted to the new test setup, the old workflows can be removed.
• Minor tweaks to various repository config files.

Note: this test setup presumes that the proposal to drop support for PHP 5.3 - see issue #145 - will be accepted as the PHPUnit Polyfills require PHP >= 5.4.

Refs:

• https://github.com/Yoast/PHPUnit-Polyfills/

TestCase: add helper methods for setup/teardown

As most tests will need to run a composer install, tests should be run in separate, temporary directories.

With that in mind, I'm adding two helper methods to the base TestCase class:

• createTestEnvironment() - this method will create a PHPCSPluginTest/TestClassName_uniqueID/local and a PHPCSPluginTest/TestClassName_uniqueID/global directory in the system temp directory and set the Composer global HOME directory to the created global directory.
• removeTestEnvironment() - this method will remove the created directories and reset the Composer global HOME directory to its original location.

Notes:

• The paths to the created directories will be added to the $tempDir, $tempGlobalPath and $tempLocalPath properties of the class and can be used in the tests.
• The methods (and associated properties) have been made static on purpose to allow for these methods to be used both in setUp()/tearDown() fixtures as well as in setUpBeforeClass()/tearDownAfterClass() fixtures. The latter would be useful for a test class needing only one environment with various test methods depending on each other and building on each other. This is also the reason that these methods are set up as helpers instead of fixtures, as this way, the decision what fixtures are most appropriate can be made for each individual concrete test class.
• A hard cleanup of the PHPCSPluginTest directory in the system temp directory has been added to the test bootstrap to make sure that any stray "old" test directories will be removed before a new test run.

Includes a static TestCase::onWindows() helper method to determine whether or not the tests are being run on Windows.

TestCase: add helper method for on-the-fly composer.json creation

This adds a test helper method, which, given an array with a Composer configuration and a target location, will on-the-fly create a composer.json file for use in a test.

The method will automatically add two additional settings to the composer.json file:

• It will add a install-codestandards script to run the plugin on demand.
• It will add the allow-plugins permission, which is needed for Composer 2.2.

By adding these keys automatically, this kind of "noise" is not needed in the actual test classes.

The method is static so as to allow for it to be used in setUpBeforeClass()/tearDownAfterClass() fixtures.

Tests: create zip package artifact for the current plugin code

In normal circumstances, a composer install will download a version of the plugin from Packagist. This will not work for the tests however, as the development version of the plugin being tested will generally not be available on Packagist.

With this in mind, we need a work-around to ensure that the tests are run with the current version of the plugin as per the last commit in the current branch.

I've implemented this by using the repositories - artifact feature of Composer.

This works as follow:

• In the test bootstrap, ahead of the test run, the plugin is zipped up as a package and placed in the tests/artifact directory.
• Whenever a composer install/update command is run within the test suite, the repositories-artifact key is injected into the composer.json when the composer.json file is written to the temp directory used for the test.
• With this in place, Composer will install the plugin from the zipped file in the tests/artifact directory instead of downloading from Packagist.

Notes:

• The version number to be used for the zip artifact of the plugin is defined in the test bootstrap file. This number may need updating once in a while to ensure it is higher than the latest released version of the plugin.
• In the CreateComposerZipArtifacts class, three properties are used to determine which files to exclude from the package - $excludedFiles, $excludedExtensions and $excludedDirs. Depending on new files/directories being introduced in the package, these lists may need to be updated once in a while.

Ref: https://getcomposer.org/doc/05-repositories.md#artifact

TestCase: add helper methods to run CLI commands and custom assertions

This commit adds six additional methods to the base TestCase class:

• executeCliCommand(): a helper method to execute a CLI command and return the exit code, the contents of stdout and the content of stderr for use in assertions.
• stabilizeCommand(): a helper method which is automatically called from the executeCliCommand() method. This method will:
  • Ensure that the PHP version which was used to initiate the test run will also be used in the CLI command. Without this, the system default PHP version would be used for CLI commands, which may be a different version from the PHP version used to run the tests and would skew the test results.
  • Ensure that the Composer version as set up using the env variable is used in the tests. Again, this will prevent the system default version of Composer being used, which may skew the results when the tests are intended to be run against a specific version of Composer.
  • Adds the --no-interaction flag to all Composer CLI commands to prevent tests hanging when interaction would be expected.
  • Adds quotes around a command when the tests are run on Windows in combination with PHP < 8 to ensure cross-platform compatibility of the CLI commands.
• getPhpcsCommand(): a helper method to get the path to the vendor installed PHPCS entry point file.
• maybeStripColors(): a helper method to compare CLI output. This method is still WIP and will probably still need some tweaking. This method will take an output expectation and an actual output and will attempt to strip CLI colour codes from the actual output when the output expectation does not contain colour codes. This way, expected colour codes can still be tested by including them in the output expectation. Note: until the bugs in this method have been smoothed out, it is recommended to use --no-ansi CLI arguments in CLI commands not testing for colour codes.
• assertExecute(): a custom assertion which will run a given CLI command, optionally in a specific working directory and will subsequently assert that the exit code is the same as expected and the stdout and stderr outputs contain the expected output. For more complex assertions against the output of CLI commands, the executeCliCommand() can be used directly in tests. The StringContainsString assertions used for stdout and stderr will attempt to strip color codes from the output before doing a compare using the maybeStripColors() method.
• assertComposerValidates(): a custom assertion to verify that a given composer.json file validates for use in the tests.

A number of these methods have been made static to allow for them to be used in conjunction with setUpBeforeClass() fixtures.

Tests: add TestListener to log and display CLI output for failing tests

While not necessarily the most pretty solution (output is displayed in the middle of the progress report), this allows for access to the composer.json content, the CLI commands and their output for any test which failed, which will be helpful when debugging test failures, especially in CI.

Tests: add initial baseline test

These tests verify:

• That the plugin can be installed and functions correctly with the full range of supported PHPCS versions. While the test is not run against the full range of supported PHPCS version each time, due to the random PHPCS version selection, all supported PHPCS versions will be tested over time in CI.
• That the plugin runs when installed.
• That the PHPCS native standards are the only recognized standards when no external standards are available.
• That no `CodeSniffer.conf``file gets created when no external standards are found.

These tests will be run in both a Composer global as well as a Composer project local environment.

The tests use a dataprovider which will randomly select two PHPCS versions compatible with the PHP version on which the test is being run + PHPCS dev-master + PHPCS 4.x (dev version for the next major).

As which PHPCS versions are compatible with which PHP versions needs quite some logic, a separate helper class PHPCSVersions is introduced, which encapsulates that knowledge.

The PHPCSVersions class contains a number of (static) methods as helpers. These can be used in any test class:

• get(): retrieves an array with a specific number of PHPCS versions valid for the current PHP version.
• getHighLow(): retrieves an array of the highest and lowest PHPCS versions valid for the current PHP version.
• getHighLowEachMajor(): retrieves an array of the highest and lowest supported PHPCS versions for each PHPCS major (and valid for the current PHP version).
• getRandom(): gets a random PHPCS version which is valid for the current PHP version.
• toDataprovider(): converts a version array to an array suitable for use as a PHPUnit dataprovider.
• getSupportedVersions(): retrieves an array with all PHPCS versions valid for the current PHP version.
• isNextMajorSupported(): determines whether the current PHP version is supported on the "next major" branch of PHPCS.
• getStandards(): retrieves an array of the names of the PHPCS native standards for a particular PHPCS version.

Generally speaking, the dev-master and 4.x branch will not be included in the retrieved ranges, but can be specifically requested by passing optional parameters.

This PHPCSVersions class will need semi-regular updates when new versions of PHPCS are released and new PHP versions are released.

Tests: move test with external standard from CI to test class

This moves the test which was being run via GH Actions to a test class in the integration test suite.

Differences between the previous and new setup:

• The previous test via GH Actions would only fail on the exit code not being 0 for any of the steps. The new PHPUnit based tests executes more detailed assertions on the results of the commands being run in the tests.
• The previous test would execute the test run of PHPCS against the src/Plugin.php file in this package. The new PHPUnit based tests will execute against a simple PHP file created on the fly.
• The previous test via GH Actions would only run for a Composer local situation. The new PHPUnit based tests cover both a Composer global as well as a local setup.
• The matrix against which the tests are run is different. In the old situation, the test would only be run on a fixed set of PHP/PHPCS combinations with Composer 2.x on Ubuntu. In the new situation, the tests will run against both Composer 1.x, as well as 2.x on both Ubuntu and Windows. Same as before, it will be run against all supported PHP versions (in the extended integrationtest workflow), but the PHPCS versions to test against are more varied and - in part - randomly selected out of the PHPCS version compatible with the current PHP version, which results in more comprehensive testing of these scenarios.

Includes:

• Adding a new createFile() helper method to the base TestCase to generate a (PHP) file to do a test run against.
• Removing the "old" integration test from the GH Actions script.

Tests: set up a fake PHPCS standard as a fixture and create an artifact of it

The tests in the RegisterExternalStandardsTest class contain quite some work-arounds to handle changes made over time to the external PHPCompatibility standard.

The PHPCompatibility standard was chosen for use in the tests as it is one of the rare standards which still supports both PHPCS 2.x as well as 3.x.

All the same, to improve the stability of the tests and to simplify the actual test code, using "fake" PHPCS standards set up as fixtures as part of this test suite will make life easier.

This commit creates the initial setup for that, by:

• Adding code to the CreateComposerZipArtifacts class to handle creating Composer package zip artifacts for each subdirectory of the tests/fixtures directory.
• Calling that code from the test bootstrap file to ensure all test artifacts are zipped up and ready for use ahead of running the tests.
• Adding an initial "fake" DummySubDir standard as a test fixture.
• Adding a README file to the tests/fixtures directory with the rationale and a list of available fixtures.

RegisterExternalStandardsTest: switch out PHPCompatibility for the test fixture

This replaces the use of the PHPCompatibility standard in the RegisterExternalStandardsTest tests with the use of the dummy-subdir fake standard fixture.

RegisterExternalStandardsTest: simplify check whether PHPCS can run with the standard

To check whether PHPCS can run with the standard, previously, a scan on a simple file was done, with this file being created on the fly for each test.

This commit replaces that check with running the "explain" command.

Advantages:

• The "explain" command does not need a file to scan and can still confirm that the standard can be read correctly by PHPCS.
• The output for the "explain" command has fewer differences across PHPCS versions, so is simpler to verify.

:new: BaseLineTest: selectively skip the tests

... in a very specific combination of circumstances - Windows + Composer 1.x + PHP 5.5 + PHPCS dev-master + no external standards -, as the Composer logs in that case, don't always show the "Running PHPCodeSniffer Composer Installer" message.

As it is such a specific combination, which will probably be rare to encounter in real life, I'm proposing to skip the tests for that combination instead of spending lots of time trying to debug this (for now).

🆕 TestCase: add willPluginOutputShow() method

In very select circumstances (PHP 5.5 with Composer 1.x on Windows) and sometimes even only with select PHPCS versions, the plugin output will not be shown in the transcript from Composer, even though the plugin does actually run.

Instead of skipping those tests completely, I'm suggestion to selectively skip the output expectation for the composer install/update run instead. That way we can still verify that the plugin has actually done its job by checking that paths have been registered with PHPCS.

I would recommend to only implement the use of this method for those select tests where we have actually seen failures due to this Composer bug and to just leave the other tests alone.

Related Issues

• https://github.com/Dealerdirect/phpcodesniffer-composer-installer/issues/9
• https://github.com/Dealerdirect/phpcodesniffer-composer-installer/issues/92

Closes #9

Problem/Motivation

phpcs is not seeing the WordPress standards.

Expected behaviour

I expect to see WordPress as one of the installed coding standards when I run phpcs -i.

Actual behaviour

WordPress does not show up in the list of installed coding standards.

Steps to reproduce

1. Create a new WordPress site with Local by Flywheel.
2. Install phpcs globally with composer global require "squizlabs/php_codesniffer=*".
3. Install the WordPress coding standards with composer create-project wp-coding-standards/wpcs --no-dev in the project's app/public directory (the root of the WordPress installation).
4. Install this package with composer require --dev dealerdirect/phpcodesniffer-composer-installer from the same directory as the previous step. Notice that it says installed_paths set to ../../../wpcs
5. Run phpcs -i and see that WordPress is not in the list.

Proposed changes

I know so little about this process that I have absolutely no proposals for what to change nor how. All I can do is give you my current setup:

OS: Windows 10 1903. WordPress: 5.2.3 PHP: 7.3.2 MySQL: 5.6.34 Composer: 1.8.6 phpcs: 3.4.2 (stable) wp-coding-standards: 2.1.1 phpcodesniffer-composer-installer: v0.5.0

I'm sure that I'm just doing something wrong, so please don't make any assumptions; ask me for any clarifications and I'll be happy to provide what I can.

As mentioned before in #113 it is desirable to move this project to an organization that has more contributors and higher visibility.

Currently PHPCSStandards looks like the best match.

This issue proposes to move the repository for this project from the Dealerdirect organisation to PHPCSStandards.

Opinions?

Proposed Changes

Improve feedback to user on failure to set paths

Related to #79.

When the setting of installed_paths fails, show an error instead of giving the impression that the setting of the installed_paths succeeded.

Use the PHP version used by Composer

Possibly not the best way to do this, but it does work, so consider this a proof of concept, if not the solution.

The getPhpExecCommand() method in the Composer\EventDispatcher\EventDispatcher class basically does what is needed: find the PHP executable used by Composer and turn it into an executable command.

Unfortunately, that method is protected, making it difficult to call that method from within the plugin.

This now copies that method into the plugin and uses it to create the command to pass on to the ProcessExecutor.

The copied method has some minor modifications to comply with the coding standards used within this project.

Based on the proof of conflict referenced in #79, I can confirm that this would solve the issue.

Related Issues

Fixes #79

Problem/Motivation

Since I use the "*" constraint in our projects, the package updated to 0.3.0 today. Right now, the default PHPCompatiblity sniff can not be found. I have not yet looked into it myself in the hope that you might know right away what the issue is.

Expected behaviour

PHPComptability sniff should still be installed

$ ./vendor/bin/phpcs -i
The installed coding standards are Zend, PHPCS, MySource, PSR2, PEAR, PSR1, Squiz and PHPCompatibility

Actual behaviour

Sniff is not installed:

$ vendor/bin/phpcs -i
The installed coding standards are Zend, PHPCS, MySource, PSR2, PEAR, PSR1 and Squiz

Steps to reproduce

composer.json:

  "require-dev": {
    "squizlabs/php_codesniffer": "^2",
    "dealerdirect/phpcodesniffer-composer-installer": "*",
    "frenck/php-compatibility": "*"
  }

Delete vendor folder (if 0.2.1 which still works was installed, the sniff is still there)

$ composer update
$ vendor/bin/phpcs 
$ ./vendor/bin/phpcs --standard=PHPCompatibility --extensions=php ./src
$ ./vendor/bin/phpcs -i

Proposed changes

Is there a migration I missed or needs frenck/PHPCompatibility a new version too?

Thanks in advance for any help.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - dealerdirect/phpcodesniffer-composer-installer v0.6.2 requires php ^5.3|^7 -> your PHP version (8.0.0) does not satisfy that requirement.
    - dealerdirect/phpcodesniffer-composer-installer v0.6.1 requires php ^5.3|^7 -> your PHP version (8.0.0) does not satisfy that requirement.
    - dealerdirect/phpcodesniffer-composer-installer v0.6.0 requires php ^5.3|^7 -> your PHP version (8.0.0) does not satisfy that requirement.
    - dealerdirect/phpcodesniffer-composer-installer v0.5.0 requires php ^5.3|^7 -> your PHP version (8.0.0) does not satisfy that requirement.

How is this invalid?

This suddenly broke a CI for me. 🤷‍♀️

Originally posted by @lilithebowman in https://github.com/Dealerdirect/phpcodesniffer-composer-installer/issues/119#issuecomment-745424644

With all of the hard work, changes and improvements over the last year (or two), we have reached a point where we can confidently say that this project has matured to a stable point.

Because of this, we will be releasing v1! 🥳🎉

This ticket is meant as a central point to coordinate what needs to be done for this release.

I think the first point in order is to decide which task belong in the v1 workload and which can be postponed until later. Non-code or non-repo changes should also be listed for thing we want to happen when the release has been deployed (like community outreach). Tickets could be made for those but that feels a bit like overkill.

To be decided

Should these be completed before the release?

• #159

Within scope for v1

These should be completed before the release.

• #160
• #188 (resolved by #191)

Todo directly after v1 release

These should be completed just after the release.

• #155

In a way a "bit late to the party", but should the default branch be renamed away from master ?

And if so, what would be the preferred new name ?

Some options:

• A lot of repos have gone with main. For a "one branch" repo, that would be an obvious choice.
• As an alternative, but that would also involve a different branching strategy, it could be considered to have a stable branch as a release branch and have a develop branch as the default PR branch. For a new release develop would then be merged into stable.

Opinions ?

Follow up on #113 and #146, which contain mentions/discussions about this as well.

Now the repository has moved to the PHPCSStandards organisation and been renamed to composer-installer, the question arises whether or not to rename the Composer package too.

Currently the Composer package name is: dealerdirect/phpcodesniffer-composer-installer

The proposed new Composer package name would be: phpcsstandards/composer-installer (in line with the new repo home)

Advantages:

• Predictability of the name/Findability of the new repo home
• Control over the package information in Packagist without a dependency on the DealerDirect organisation.

Disadvantages:

• Loss of the statistics on Packagist (24M downloads and counting)
• Code churn for the 30.000+ repositories using this package. It may take years before all users have moved over ~~and users may not benefit of improvements made in the package until they have updated their composer.json config~~ (looks like the old package will still update).
  • Each of these would need to "allow" the new plugin for Composer 2.2+.
  • For those repositories for which this package is a root dependency, the dependency would need to be updated too.

If it would be decided to rename the package, this link was mentioned by @GaryJones in #113 containing information on how to manage this on the Packagist side of things: https://github.com/composer/packagist/issues/47

Tasks if the package would be renamed:

• [ ] See https://github.com/composer/packagist/issues/47 for the Packagist side of things
• [ ] Fix/update all references to the package name in the repo
• [ ] Update the namespace of the code ?

I'd like to re-open the discussion around whether the paths registered with PHPCS should be absolute or relative paths.

Some previous issues related to this: #14, #33, #73

Current status after #28:

• Global installs use absolute paths
• Local installs use relative paths

Problems I'm seeing

• In some tests on Windows, I'm seeing duplicate relative paths being registered, i.e.:

  Using config file: C:\\Users\\runneradmin\\AppData\\Local\\Temp\\PHPCSPluginTest\\RegisterExternalStandardsTest_61fe56765bc692.23339840\\local\\vendor\\squizlabs\\php_codesniffer\\CodeSniffer.conf
  
  installed_paths: ../../../../../../../../../../RUNNER~1/AppData/Local/Temp/PHPCSPluginTest/RegisterExternalStandardsTest_61fe56765bc692.23339840/local/vendor/phpcs-composer-installer,../../phpcs-composer-installer
  

  As this will result in the same standard(s) being registered twice with PHPCS, this is likely to cause problems with the autoloading of sniffs.

• Even without duplicate paths, I've had reports of relative paths causing problems with the PHPCS autoloading of sniffs: https://github.com/PHPCompatibility/PHPCompatibility/issues/1311#issuecomment-1011697527

Proposal

Always use absolute paths.

Further research needed

• What about projects in a dual OS setup, like Windows with WSL or a Vagrant environment ?
• What about projects with a committed vendor directory ? => I'd like to suggest that they can add a script to run this plugin to their composer.json and can run that script whenever the project is checked out. After all, the plugin will update the paths based on the runtime environment, so should fix the paths in the committed CodeSniffer.Conf file to the new environment.
• Can this be made easier ? => In Composer 2.x, Plugins can add their own commands to Composer. We could consider adding a custom command to run this plugin. This would negate the need to add the script. Ref: https://getcomposer.org/doc/articles/plugins.md#command-provider
• Should there still be an option for users to use relative paths ? => Along the same lines as the search-depth option, we could consider adding a force-relative-paths option. We should probably also investigate if custom commands can take custom arguments. In that case, we could possible add support for a --force-relative-paths CLI argument if needs be.

At this moment, the plugin very deliberately only registers packages with the phpcodesniffer-standard type set in their composer.json file with PHP_CodeSniffer (providing a ruleset.xml file can be found in the package).

There are currently 307 packages registered in Packagist which use the phpcodesniffer-standard type.

When going through Packagist, however, there are a number of other "types" typically used for PHPCS standards:

• coding-standard: 24 packages
• phpcs-standard: 7 packages - most seem to be abandoned, but all seem to be expect to be used with the Goatherd PHPCS Installer (Composer plugin) which seems to have been abandoned 7 years ago
• coding-standards: 1 package - is actually a CS-Fixer package
• phpcs-coding-standard: 1 package - PHPCS 1.x package and looks abandoned
• phpcs-coding-standards: . package - candidate to suggest changing the type
• phpcs-ruleset: 1 package - candidate to suggest changing the type
• codesniffer-standard: 1 package - candidate to suggest changing the type

If I remember correctly, a couple of years ago, a round of PRs was send in to repos for external PHPCS standards to update the Composer "type". Would it be an idea to selectively do so again ?