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
XML Documentation xsd and samples how it could look like #2
Conversation
…inst / work with to easy the writing of the xml
@@ -14,11 +14,10 @@ | |||
*/ | |||
|
|||
/** | |||
* Generic_Sniffs_Files_LineLengthSniff. | |||
* Checks all lines in the file and warnings and errors they are to long |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be: Checks all lines in the file and shows warnings or errors if they are too long
Besides the above text changes, I really like where this is going. If we can agree on the wording, we're good to go. Although I still need to write some code to substitue those keywords with their correct values based on the standard the user selected. |
I'm sorry about the bad wordings. I didn't proofread this even so i really should have :) If you want to note the ability to change those values on every sniff that has changeable configuration I'm fine with that. I'm aware of the abilty to change the values in the ruleset file.. i didn't think about extending the sniff classes to be honest as the xml file is all I ever used :) Let me know if you find other issues or pull and I'll get started on writing something for every sniff in generic |
<standard> | ||
<![CDATA[ | ||
It is recommended to keep lines at approximately 80 characters long for better code readability. | ||
It is recommended to keep lines at approximately {lineLimit} characters long for better code readability and lines should never exceed {absoluteLineLimit} characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking more about it, this isn't gong to work. If you have disabled your absoluteLineLimit (set it to zero) then the message will show:
"It is recommended to keep lines at approximately 80 characters long for better code readability and lines should never exceed 0 characters."
We obviously need to devise a smarter system that allows documentation to be generated based on the values that the project-specific standard is setting.
I think this is more complex than a pull request is going to achieve. I think we need to start again on this and plan it out properly. It requires changes to the existing documentation generator code as well as changes to the documentation itself. I also wont be pulling this in if it stops existing documentation from being generated, which it obviously will due to the need to make core changes that have not yet been done. If you'd like to start creating examples and basic documentation for the existing sniffs using the existing XML format, I'd be very happy to accept those contributions. But I'll need to have a bit more of a think about how best create customised standard documentation. |
Anyways, thanks for getting back to me on this :) |
Relax class name req's; we're not yet namespacing
…h and open files in that during interactive session
…h and open files in that during interactive session
…is incorrectly quoted
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
…h and open files in that during interactive session
The `Fixer::generateDiff()` method calls the `shell_exec()` method to retrieve the `diff` between two files. In the unit tests, this is used to compare the expected content in a `.fixed` file with the generate fixed file contents. If the expected content matches the generated content, the diff command will produce no output and `shell_exec()` will return `null`. Ref: https://www.php.net/manual/en/function.shell-exec.php This result is subsequently passed to `explode()` as the second parameter, but `explode()` only excepts strings as the second parameter. Ref: https://www.php.net/manual/en/function.explode As of PHP 8.1, this will generate a deprecation notice `explode(): Passing null to parameter squizlabs#2 ($string) of type string is deprecated`. Discovered while testing an external standard against PHPCS `master` on PHP 8.1. Fixed now.
The `Fixer::generateDiff()` method calls the `shell_exec()` method to retrieve the `diff` between two files. In the unit tests, this is used to compare the expected content in a `.fixed` file with the generate fixed file contents. If the expected content matches the generated content, the diff command will produce no output and `shell_exec()` will return `null`. Ref: https://www.php.net/manual/en/function.shell-exec.php This result is subsequently passed to `explode()` as the second parameter, but `explode()` only excepts strings as the second parameter. Ref: https://www.php.net/manual/en/function.explode As of PHP 8.1, this will generate a deprecation notice `explode(): Passing null to parameter #2 ($string) of type string is deprecated`. Discovered while testing an external standard against PHPCS `master` on PHP 8.1. Fixed now.
…le bug The `DummyFile::__construct()` method expects a `string` as the first parameter, subsequently passes it to the `File::setContent()` method, which also expects a `string`, which then passes it to the `Common::detectLineEndings()` method, which again expects a `string`. Within the `Common::detectLineEndings()` method, the string is then passed to the PHP native `preg_match()` function, which (again) expects as `string` for the `$subject` parameter. When using PHPCS with parallel processing turned on, on a system which allows for parallel processing, the `DummyFile` class was, however, being instantiated in the `Runner::processChildProcs()` method with `null` as the content of the first parameter, which on PHP 8.1 leads to `preg_match(): Passing null to parameter squizlabs#2 ($subject) of type string is deprecated` deprecation notices. This deprecation notice was then caught as a `RuntimeException` in the `File::setContent()` method and passed on to the `File::addWarningOnLine()`, which called the `File::addMessage()` method. The `File::addMessage()` parameter then ran into trouble as the `$this->config` property has not been set up yet, as `DummyFile::__construct()` calls `File::setContent()` before calling the `parent::__construct()` method, leading to the `Undefined array key "cache"` notices which were making the build fail. Fixed now by passing an empty string instead of `null` as the `$content` for the `DummyFile` in the `Runner::processChildProcs()` method. This then leaves one more issue: the `DummyFile::__construct()` method contains a conditional code block which was only run when `$content !== null`. As this conditional code block is also not necessary to be run when an empty string would be passed to the constructor, changing this condition to `$content !== ''` makes that the condition can still match and maintains the efficiency tweak the condition was safeguarding.
…le bug The `DummyFile::__construct()` method expects a `string` as the first parameter, subsequently passes it to the `File::setContent()` method, which also expects a `string`, which then passes it to the `Common::detectLineEndings()` method, which again expects a `string`. Within the `Common::detectLineEndings()` method, the string is then passed to the PHP native `preg_match()` function, which (again) expects as `string` for the `$subject` parameter. When using PHPCS with parallel processing turned on, on a system which allows for parallel processing, the `DummyFile` class was, however, being instantiated in the `Runner::processChildProcs()` method with `null` as the content of the first parameter, which on PHP 8.1 leads to `preg_match(): Passing null to parameter squizlabs#2 ($subject) of type string is deprecated` deprecation notices. This deprecation notice was then caught as a `RuntimeException` in the `File::setContent()` method and passed on to the `File::addWarningOnLine()`, which called the `File::addMessage()` method. The `File::addMessage()` parameter then ran into trouble as the `$this->config` property has not been set up yet, as `DummyFile::__construct()` calls `File::setContent()` before calling the `parent::__construct()` method, leading to the `Undefined array key "cache"` notices which were making the build fail. Fixed now by passing an empty string instead of `null` as the `$content` for the `DummyFile` in the `Runner::processChildProcs()` method. This then leaves one more issue: the `DummyFile::__construct()` method contains a conditional code block which was only run when `$content !== null`. As this conditional code block is also not necessary to be run when an empty string would be passed to the constructor, changing this condition to `$content !== ''` makes that the condition can still match and maintains the efficiency tweak the condition was safeguarding.
…le bug The `DummyFile::__construct()` method expects a `string` as the first parameter, subsequently passes it to the `File::setContent()` method, which also expects a `string`, which then passes it to the `Common::detectLineEndings()` method, which again expects a `string`. Within the `Common::detectLineEndings()` method, the string is then passed to the PHP native `preg_match()` function, which (again) expects as `string` for the `$subject` parameter. When using PHPCS with parallel processing turned on, on a system which allows for parallel processing, the `DummyFile` class was, however, being instantiated in the `Runner::processChildProcs()` method with `null` as the content of the first parameter, which on PHP 8.1 leads to `preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated` deprecation notices. This deprecation notice was then caught as a `RuntimeException` in the `File::setContent()` method and passed on to the `File::addWarningOnLine()`, which called the `File::addMessage()` method. The `File::addMessage()` parameter then ran into trouble as the `$this->config` property has not been set up yet, as `DummyFile::__construct()` calls `File::setContent()` before calling the `parent::__construct()` method, leading to the `Undefined array key "cache"` notices which were making the build fail. Fixed now by passing an empty string instead of `null` as the `$content` for the `DummyFile` in the `Runner::processChildProcs()` method. This then leaves one more issue: the `DummyFile::__construct()` method contains a conditional code block which was only run when `$content !== null`. As this conditional code block is also not necessary to be run when an empty string would be passed to the constructor, changing this condition to `$content !== ''` makes that the condition can still match and maintains the efficiency tweak the condition was safeguarding.
in some analysis, I was receiving this error "Uncaught TypeError: vsprintf(): Argument squizlabs#2 ($values) must be of type array, string given" on line 1056
Continuing the discussion I had with @gsherwood in #1
I needed recreate the PR because github refused to update the range.