Skip to content

Commit a97cfb4

Browse files
jrfnljrfnl
authored
Merge pull request #40 from PHPCSStandards/feature/contributing-improvements
Improve available CONTRIBUTING information and guidelines
2 parents d956293 + d464dfb commit a97cfb4

File tree

5 files changed

+260
-16
lines changed

5 files changed

+260
-16
lines changed

‎.github/CONTRIBUTING.md

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
Contributing
2+
-------------
3+
4+
Thank you for your interest in contributing to PHP_CodeSniffer!
5+
6+
7+
## Reporting Bugs
8+
9+
Please search the [open issues](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues) to see if your issue has been reported already and if so, comment in that issue if you have additional information, instead of opening a new one.
10+
11+
Before reporting a bug, you should check what sniff an error is coming from.
12+
Running `phpcs` with the `-s` flag will show the name of the sniff for each error.
13+
14+
If the error code starts with anything other than `Generic`, `MySource`, `PEAR`, `PSR1`, `PSR2`, `PSR12`, `Squiz` or `Zend`, the error is likely coming from an external PHP_CodeSniffer standard.
15+
**Please report bugs for externally maintained sniffs to the appropriate repository.**
16+
17+
Bug reports containing a minimal code sample which can be used to reproduce the issue are highly appreciated as those are most easily actionable.
18+
19+
:point_right: Reports which only include a _screenshot_ of the code will be closed without hesitation as not actionable.
20+
21+
22+
### Reporting Security Issues
23+
24+
PHP_CodeSniffer is a developer tool and should generally not be used in a production environment.
25+
26+
Having said that, responsible disclosure of security issues is highly appreciated.
27+
Issues can be reported privately to the maintainers by opening a [Security vulnerability report](https://github.com/PHPCSStandards/PHP_CodeSniffer/security/advisories/new).
28+
29+
30+
### Support/Questions About Using PHP_CodeSniffer
31+
32+
Please read the [documentation in the wiki](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki) before opening an issue with a support question.
33+
34+
The [discussion forum](https://github.com/PHPCSStandards/PHP_CodeSniffer/discussions) or [StackOverflow](https://stackoverflow.com/questions/tagged/phpcodesniffer) are the appropriate places for support questions.
35+
36+
37+
## Contributing Without Writing Code
38+
39+
### Issue Triage
40+
41+
We welcome issue triage.
42+
43+
Issue triage is the action of verifying a reported issue is reproducible and is actually an issue.
44+
This includes checking whether the issue is something which should be fixed in **_this_** repository.
45+
46+
To find issues which need triage, look for [issues without labels](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues?q=is%3Aopen+is%3Aissue+no%3Alabel) or issues with the ["Status: triage"](https://github.com/PHPCSStandards/PHP_CodeSniffer/labels/Status%3A%20triage) label.
47+
48+
#### Typical issue triage tasks:
49+
* Verify whether the issue is reproducible with the given information.
50+
* Ask for additional information if it is not.
51+
* If you find the issue is reported to the wrong repo, ask the reporter to report it to the correct external standard repo and suggest closing the issue.
52+
53+
Additionally for older issues:
54+
* Check whether an issue still exists or has been fixed in `master` since the issue was initially reported.
55+
* If it has been fixed, document (in a comment) which commit/PR was responsible for fixing the issue and suggest closing the ticket.
56+
57+
58+
### Testing Open Pull Requests
59+
60+
Testing pull requests to verify they fix the issue they are supposed to fix without side-effects is an important task.
61+
62+
To get access to a PHPCS version which includes the patch from a pull request, you can:
63+
* Either use a git clone of the PHP_CodeSniffer repository and check out the PR.
64+
* Or download the PHAR file(s) for the PR, which is available from the [Test workflow](https://github.com/PHPCSStandards/PHP_CodeSniffer/actions/workflows/test.yml) as an artifact of the workflow run.
65+
The PHAR files can be found on the summary page of the test workflow run for the PR.
66+
If the workflow has not been run (yet), the PHAR artifact may not be available (yet).
67+
68+
#### Typical test tasks:
69+
* Verify that the patch solves the originally reported problem.
70+
* Verify that the tests added in the PR fail without the fix and pass with the fix.
71+
* For a fix for false negatives: verify that the correct error message(s) are thrown by the patched code.
72+
* Run the patched PHPCS version against real codebases to see if the fix creates any side-effects (new false positives/false negatives).
73+
74+
75+
### Writing sniff documentation
76+
77+
Sniffs in PHP_CodeSniffer should preferably be accompanied by documentation. There is currently still a lot of documentation missing.
78+
79+
Sniff documentation is provided via XML files in the standard's `Docs` directory and is available to end-users via the command-line and/or can be compiled into an HTML or Markdown file.
80+
81+
To see an example of some of the available documentation, run:
82+
```bash
83+
phpcs --standard=PSR12 --generator=Text
84+
```
85+
86+
Pull requests to update existing documentation, or to add documentation for sniffs which currently don't have any, are welcome.
87+
88+
For the documentation to be recognized, the naming conventions have to be followed.
89+
90+
For example, for the sniff named `Generic.NamingConventions.ConstructorName`:
91+
* The sniff lives in the `src/Standards/Generic/Sniffs/NamingConventions/ConstructorNameSniff.php` file.
92+
* The associated documentation should be in a `src/Standards/Generic/Docs/NamingConventions/ConstructorNameStandard.xml` file.
93+
94+
The XML files consist of several parts:
95+
* The `title` attribute in the `<documentation>` tag should generally reflect the name of the sniff.
96+
* Each XML file can contain multiple `<standard>` blocks.
97+
* Each `<standard>` block can be accompanied by one or more `<code_comparison>` blocks.
98+
* Each code comparison block should have two `<code>` blocks, the first one showing "valid" code, the second one showing "invalid" code.
99+
100+
Some guidelines to get you started:
101+
* Keep it as simple as possible.
102+
* When a sniff shows multiple different errors/warnings, it is recommended to have a separate `<standard>` block for each error/warning.
103+
* The title of a "good" code sample (on the left) should start with `Valid:`.
104+
* The title of a "bad" code sample (on the right) should start with `Invalid:`.
105+
* Don't use example code which can be traced back to a specific project.
106+
* Each line within the code sample should be max 48 characters.
107+
* Code used in code samples should be valid PHP.
108+
* To highlight the "good" and the "bad" bits in the code examples, surround those bits with `<em> ...</em>` tags.
109+
These will be removed automatically when using the text generator, but ensure highlighting of the code in Markdown/HTML.
110+
* The indentation in the XML file should use spaces only. Four spaces for each indent.
111+
112+
Make sure to test the documentation before submitting a PR by running:
113+
```bash
114+
phpcs --standard=StandardName --generator=Text --sniffs=StandardName.Category.SniffName
115+
```
116+
117+
Kind request to add only one new XML file per PR to make the PR easier to review.
118+
119+
120+
## Contributing With Code
121+
122+
### Requesting/Submitting New Features
123+
124+
Ideally, start by [opening an issue](https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/new/choose) to check whether the feature is something which is suitable to be included in PHP_CodeSniffer.
125+
126+
It's possible that a feature may be rejected at an early stage, and it's better to find out before you write the code.
127+
128+
It is also good to discuss potential different implementation options ahead of time and get guidance for the preferred option from the maintainers.
129+
130+
Note: There may be an issue or PR open already. If so, please join the discussion in that issue or PR instead of opening a duplicate issue/PR.
131+
132+
> Please note: Auto-fixers will only be accepted for "non-risky" sniffs.
133+
> If a fixer could cause parse errors or a change in the behaviour of the scanned code, the fixer will **NOT** be accepted in PHP_CodeSniffer and may be better suited to an external standard.
134+
135+
136+
### Getting started
137+
138+
1. Fork/clone the repository.
139+
2. Run `composer install`.
140+
When installing on PHP >= 8.0, use `composer install --ignore-platform-req=php+`.
141+
3. Create a new branch off the `master` branch to hold your patch.
142+
If there is an open issue associated with your patch, including the issue number in the branch name is good practice.
143+
144+
145+
### While working on a patch
146+
147+
Please make sure your code conforms to the PHPCS coding standard, is covered by tests and that all the PHP_CodeSniffer unit tests still pass.
148+
149+
Also, please make sure your code is compatible with the PHP_CodeSniffer minimum supported PHP version, PHP 5.4.
150+
151+
To help you with this, a number of convenience scripts are available:
152+
* `composer check-all` will run the `cs` + `test` checks in one go.
153+
* `composer cs` will check for code style violations.
154+
* `composer cbf` will run the autofixers for code style violations.
155+
* `composer test` will run the unit tests (only works when on PHP < 8.1).
156+
* `composer test-php8` will run the unit tests when you are working on PHP 8.1+.
157+
Please note that using a `phpunit.xml` overload config file will not work with this script!
158+
* `composer coverage` will run the unit tests with code coverage (only works when on PHP < 8.1).
159+
Note: you may want to use a custom `phpunit.xml` overload config file to tell PHPUnit where to place an HTML report.
160+
Alternative run it like so: `composer coverage -- --coverage-html /path/to/report-dir/` to specify the location for the HTML report on the command line.
161+
* `composer build` will build the phpcs.phar and phpcbf.phar files.
162+
163+
N.B.: You can ignore any skipped tests as these are for external tools.
164+
165+
166+
### Writing tests
167+
168+
Tests for the PHP_CodeSniffer engine can be found in the `tests/Core` directory.
169+
Tests for individual sniffs can be found in the `src/Standards/[StandardName]/Tests/[Category]/` directory.
170+
171+
Tests will, in most cases, consist of a set of related files and follow strict naming conventions.
172+
173+
For example, for the sniff named `Generic.NamingConventions.ConstructorName`:
174+
* The sniff lives in the `src/Standards/Generic/Sniffs/NamingConventions/` directory.
175+
* The tests live in the `src/Standards/Generic/Tests/NamingConventions/` directory.
176+
* The tests consist of two files:
177+
- `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.inc` which is the test _case_ file containing code for the sniff to analyse.
178+
- `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.php` which is the test file, containing two methods, `getErrorList()` and `getWarningList()`, which should each return an array with as the keys the line number in the test _case_ file and as the values the number of errors or warnings which are expected on each line.
179+
Only lines on which errors/warnings are expected need to be included in the lists. All other lines will automatically be set to expect no errors and no warnings.
180+
181+
#### Multiple test case files
182+
183+
At times, one test _case_ file is not enough, for instance when the sniff needs to behave differently depending on whether or code is namespaced or not, or when a sniff needs to check something at the top of a file.
184+
185+
The test framework allows for multiple test _case_ files.
186+
In that case, the files should be numbered and the number should be placed between the file name and the extension.
187+
188+
So for the above example, the `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.inc` would be renamed to `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.1.inc` and additional test case files should be numbered sequentially like `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.2.inc`, `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.3.inc` etc.
189+
190+
The `getErrorList()` and the `getWarningList()` methods will receive an optional `$testFile=''` parameter with the file name of the file being scanned - `ConstructorNameUnitTest.2.inc` - and should return the appropriate array for each file.
191+
192+
#### Testing fixers
193+
194+
If a sniff contains errors/warnings which can be auto-fixed, these fixers should also be tested.
195+
196+
This is done by adding an additional test _case_ file with an extra `.fixed` extension for each test _case_ file which expects fixes.
197+
198+
For example, if the above `Generic.NamingConventions.ConstructorName` would contain an auto-fixer, there should be an additional `src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.inc.fixed` file containing the code as it is expected to be after the fixer has run.
199+
200+
The test framework will compare the actual fixes made with the expected fixes and will fail the tests if these don't match.
201+
202+
### Submitting your pull request
203+
204+
Some guidelines for submitting pull requests (PRs) and improving the chance that your PR will be merged:
205+
* Please keep your PR as small as possible, but no smaller than that.
206+
If your change is complex, make sure you add a proper commit message explaining what you have done and why.
207+
* Please clean up your branch before pulling your PR.
208+
Small PRs using atomic, descriptive commits are hugely appreciated as it will make reviewing your changes easier for the maintainers.
209+
* Only open your PR when you've finished work on it and you are confident that it is ready for review.
210+
* Please make sure your pull request passes all continuous integration checks.
211+
PRs which are failing their CI checks will likely be ignored by the maintainers or closed.
212+
* Please respond to PR reviews in a timely manner.
213+
214+
Your time is valuable and we appreciate your willingness to spend it on this project.
215+
However, the maintainers time is also valuable and often, more scarce, so please be considerate of that.
216+
217+
## Licensing
218+
219+
By contributing code to this repository, you agree to license your code for use under the [BSD-3-Clause license](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt).

‎.github/pull_request_template.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Fixes #
3232
## PR checklist
3333
<!-- Go over all the following points, and put an `x` in all the boxes that apply. -->
3434
- [ ] I have checked there is no other PR open for the same change.
35-
- [ ] I have read the [Contribution Guidelines](.github/CONTRIBUTING.md).
35+
- [ ] I have read the [Contribution Guidelines](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/.github/CONTRIBUTING.md).
3636
- [ ] I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
3737
- [ ] I have added tests to cover my changes.
3838
- [ ] I have verified that the code complies with the projects coding standards.

‎CONTRIBUTING.md

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Bug reports and feature requests can be submitted on the [Github Issue Tracker](
9999

100100
## Contributing
101101

102-
See [CONTRIBUTING.md](CONTRIBUTING.md) for information.
102+
See [CONTRIBUTING.md](.github/CONTRIBUTING.md) for information.
103103

104104
## Versioning
105105

‎composer.json

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,43 @@
4848
"bin": [
4949
"bin/phpcs",
5050
"bin/phpcbf"
51-
]
51+
],
52+
"scripts": {
53+
"cs": [
54+
"@php ./bin/phpcs"
55+
],
56+
"cbf": [
57+
"@php ./bin/phpcbf"
58+
],
59+
"test": [
60+
"Composer\\Config::disableProcessTimeout",
61+
"@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php --no-coverage"
62+
],
63+
"test-php8": [
64+
"Composer\\Config::disableProcessTimeout",
65+
"@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php --no-configuration --bootstrap=tests/bootstrap.php --dont-report-useless-tests --no-coverage"
66+
],
67+
"coverage": [
68+
"Composer\\Config::disableProcessTimeout",
69+
"@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php -d max_execution_time=0"
70+
],
71+
"build": [
72+
"Composer\\Config::disableProcessTimeout",
73+
"@php -d phar.readonly=0 -f ./scripts/build-phar.php"
74+
],
75+
"check-all": [
76+
"@cs",
77+
"@test",
78+
"@check-package"
79+
]
80+
},
81+
"scripts-descriptions": {
82+
"cs": "Check for code style violations.",
83+
"cbf": "Fix code style violations.",
84+
"test": "Run the unit tests without code coverage.",
85+
"test-php8": "Run the unit tests without code coverage on PHP 8.1 or higher.",
86+
"coverage": "Run the unit tests with code coverage.",
87+
"build": "Create PHAR files for PHPCS and PHPCBF.",
88+
"check-all": "Run all checks (phpcs, tests)."
89+
}
5290
}

0 commit comments

Comments
 (0)