Skip to content
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

[FEATURE] Add support to import namespaces for sniffs #25

Closed
wants to merge 1 commit into from
Closed

[FEATURE] Add support to import namespaces for sniffs #25

wants to merge 1 commit into from

Conversation

andygrunwald
Copy link
Contributor

During the development of PHP_CodeSniffer-Sniffs for the CMS TYPO3 and the framework FLOW3 we create some own codesniffs. Our folder structure looks like:

|-TYPO3
|--Sniffs
|-TYPO3v4
|--ruleset.xml
|-FLOW3
|--ruleset.xml

Folder "TYPO3" contains all sniffs (Sniffpool). TYPO3v4 and FLOW3 implement only a subset of the sniffs, which are located in TYPO3. At the moment we have to symlink the TYPO3-Folder into the PHP_CodeSniffer standard directory to use this standard. This is not a nice solution.

This pull request try to fix this problem. This commit introduces the possibility to import sniff namespaces. Imagine you add your rules like

<rule ref="TYPO3.Files.OneClassPerFile" />

in your ruleset.xml. If there is no symlink in your standard directory for TYPO3 during execution of your ruleset an exception will be thrown. With this commit you have the possibility to import the namespace TYPO3 like

<namespace-import name="TYPO3" ref="../TYPO3/" />

Now the TYPO3 standard is known without symlinking :)

This feature makes it more easier for us and more independent from the standard directory.
What do you think about this? Your feedback is very welcome.

During the development of sniffs for the CMS TYPO3 and the
framework FLOW3 we create some own codesniffs. Out folder
structure looks like:

|-TYPO3
|--Sniffs
|-TYPO3v4
|--ruleset.xml
|-FLOW3
|--ruleset.xml

Folder TYPO3 contains all sniffs. TYPO3v4 and FLOW3 implement
only a subset of the sniffs, which are located in FLOW3.
At the moment we have to symlink the TYPO3-Folder into the
PHP_CodeSniffer standard directory to use this. This
is not a nice solution.

This commit try to fix this problem. This commit introduces
to import sniff namespaces. Imagine you add your rules like
<rule ref="TYPO3.Files.OneClassPerFile" />

If there is no symlink in your standard directory for TYPO3
an exception will be thrown. With this commit you have the
possibility to import the namespace TYPO3 like
<namespace-import name="TYPO3" ref="../TYPO3/" />

Now the TYPO3 standard is known without symlinking :)
@gsherwood
Copy link
Member

I really like this feature, but there are similar features already supported in the ruleset.xml format. You can load a standard by specifying the path, or a directory of sniffs, or a single sniff. The only problem is that it doesn't currently support relative paths.

So maybe instead of adding a new namespace tag that has relative path support, I should just change the existing RULE tag to support relative paths. So then you'd include the sniffs you want like:

 <rule ref="../TYPO3/Sniffs/Files/OneClassPerFileSniff.php" />

or for all sniffs in the Files dir

<rule ref="../TYPO3/Sniffs/Files" />

or if it is easier to include the whole TYPO3 standard and just exclude some sniffs:

<rule ref="../TYPO3" />
(and then set severity of various messages to 0)

@gsherwood
Copy link
Member

By the way, I'm going to implement the relative path feature anyway because it would be good to have. It's also a pretty easy change given refs can't start with a "." (except if your are using a hidden dir I guess) and so I can assume this is a relative path. If it isn't and realpath() fails, I can just try the fallback code.

Commit is here: e329880

@andygrunwald
Copy link
Contributor Author

Hey,

first of all thanks for taking care of my "wish" / "issue".
I like your idea to reuse the existing features and make them "complete" to support relative pathes. This is much better than adding a new tag like "namespace-import".

It is awesome that you have implemented the relative path feature so quick. Thanks for that.
I have tried to test your feature with our setup and i was not successfull :(
In the following lines i try to explain the problem and my solution.

  • Clone the latest master of https://github.com/squizlabs/PHP_CodeSniffer
  • Clone the latest master of git://git.typo3.org/Teams/forge.typo3.org/hudson-helpers/tools/PHP_CodeSniffer.git (Our TYPO3 FLOW3 Sniffs)
  • Change the TYPO3v4/ruleset.xml to the following content
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="TYPO3v4">
    <description>The coding standard for TYPO3 Version 4 / 6. Have a look at http://forge.typo3.org/projects/team-php_codesniffer/wiki</description>

    <!-- Arrays -->
    <rule ref="Squiz.Arrays.ArrayBracketSpacing" />

    <!-- Files -->
    <rule ref="../TYPO3/Sniffs/Files/OneClassPerFileSniff.php" />
    <rule ref="../TYPO3/Sniffs/Files/LowercasedFilenameSniff.php" />
    <rule ref="../TYPO3/Sniffs/Files/EncodingUtf8Sniff.php" />
    <rule ref="../TYPO3/Sniffs/Files/OneInterfacePerFileSniff.php" />
    <rule ref="Generic.Files.LineEndings" />
    <rule ref="Generic.Files.LineLength">
        <properties>
            <property name="lineLimit" value="130"/>
            <property name="absoluteLineLimit" value="200"/>
        </properties>
    </rule>
    <rule ref="../TYPO3/Sniffs/Files/IncludingFileSniff.php" />

    <!-- Classes -->
    <rule ref="../TYPO3/Sniffs/Classes" />
    <rule ref="Squiz.Classes.SelfMemberReference" />
</ruleset>
  • Execute the command to process the sniffs (of course change the Folder "TestSniffs" to your TYPO3v4 clone)
php ./PHP_CodeSniffer/scripts/phpcs --standard=./TestSniffs/TYPO3v4/ruleset.xml class.t3lib_befunc.php

On my machine, the following error occured:

PHP Fatal error:  Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist' in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:852
Stack trace:
#0 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(750): PHP_CodeSniffer->_expandRulesetReference(Object(SimpleXMLElement))
#1 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(643): PHP_CodeSniffer->getSniffFiles('/Users/andygrun...', 'TYPO3v4')
#2 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(448): PHP_CodeSniffer->setTokenListeners('./cs-test/TYPO3...', Array)
#3 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer/CLI.php(545): PHP_CodeSniffer->process(Array, './cs-test/TYPO3...', Array, false)
#4 /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs(37): PHP_CodeSniffer_CLI->process()
#5 {main}
  thrown in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

Fatal error: Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist' in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

PHP_CodeSniffer_Exception: Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

Call Stack:
    0.0148     633800   1. {main}() /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs:0
    0.0643    2277488   2. PHP_CodeSniffer_CLI->process() /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs:37
    0.0649    2285200   3. PHP_CodeSniffer->process() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer/CLI.php:545
    0.0649    2286840   4. PHP_CodeSniffer->setTokenListeners() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:448
    0.0888    2287976   5. PHP_CodeSniffer->getSniffFiles() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:643
    0.0901    2296152   6. PHP_CodeSniffer->_expandRulesetReference() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:750

My solution was to change the realpath-line in your commit (e329880) to

$realpath = realpath(self::$standardDir.'/'.$sniff);

Just remove the "dirname()" call.
After this change, it just works fine :)

If you need help to reproduce my error, just drop me a line and i will setup a demo.
Or is it my fault and only a small miss configuration?

After fixing this "bug" / "feature" (?) my pull request is obsolete and can be closed, of course.
THX for taking care of it again.

@gsherwood
Copy link
Member

Hi Andy,

I've updated the code to do an additional check because the
standardDir is sometimes a path to an xml file (like is my test cases)
and sometimes a path to a directory (like in your case). Please try
re-cloning PHPCS to give it a test. I converted all your paths in the
ruleset.xml file for testing and all sniffs seem to be working.

Thanks for the great test instructions.

Greg

On Sat, Apr 28, 2012 at 1:33 AM, Andy Grunwald
reply@reply.github.com
wrote:

Hey,

first of all thanks for taking care of my "wish" / "issue".
I like your idea to reuse the existing features and make them "complete" to support relative pathes. This is much better than adding a new tag like "namespace-import".

It is awesome that you have implemented the relative path feature so quick. Thanks for that.
Ive tried to test your feature with our setup and i wasnt successfull :(
In the following lines i try to explain the problem and my solution.

  • Clone the latest master of https://github.com/squizlabs/PHP_CodeSniffer
  • Clone the latest master of git://git.typo3.org/Teams/forge.typo3.org/hudson-helpers/tools/PHP_CodeSniffer.git (Our TYPO3 FLOW3 Sniffs)
  • Change the TYPO3v4/ruleset.xml to the following content
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="TYPO3v4">
       <description>The coding standard for TYPO3 Version 4 / 6. Have a look at http://forge.typo3.org/projects/team-php_codesniffer/wiki</description>

       <!-- Arrays -->
       <rule ref="Squiz.Arrays.ArrayBracketSpacing" />

       <!-- Files -->
       <rule ref="../TYPO3/Sniffs/Files/OneClassPerFileSniff.php" />
       <rule ref="../TYPO3/Sniffs/Files/LowercasedFilenameSniff.php" />
       <rule ref="../TYPO3/Sniffs/Files/EncodingUtf8Sniff.php" />
       <rule ref="../TYPO3/Sniffs/Files/OneInterfacePerFileSniff.php" />
       <rule ref="Generic.Files.LineEndings" />
       <rule ref="Generic.Files.LineLength">
               <properties>
                       <property name="lineLimit" value="130"/>
                       <property name="absoluteLineLimit" value="200"/>
               </properties>
       </rule>
       <rule ref="../TYPO3/Sniffs/Files/IncludingFileSniff.php" />

       <!-- Classes -->
       <rule ref="../TYPO3/Sniffs/Classes" />
       <rule ref="Squiz.Classes.SelfMemberReference" />
</ruleset>
  • Execute the command to process the sniffs (of course change the Folder "TestSniffs" to your TYPO3v4 clone)
php ./PHP_CodeSniffer/scripts/phpcs --standard=./TestSniffs/TYPO3v4/ruleset.xml class.t3lib_befunc.php

On my machine, the following error occured:

PHP Fatal error:  Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist' in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:852
Stack trace:
#0 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(750): PHP_CodeSniffer->_expandRulesetReference(Object(SimpleXMLElement))
#1 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(643): PHP_CodeSniffer->getSniffFiles('/Users/andygrun...', 'TYPO3v4')
#2 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php(448): PHP_CodeSniffer->setTokenListeners('./cs-test/TYPO3...', Array)
#3 /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer/CLI.php(545): PHP_CodeSniffer->process(Array, './cs-test/TYPO3...', Array, false)
#4 /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs(37): PHP_CodeSniffer_CLI->process()
#5 {main}
 thrown in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

Fatal error: Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist' in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

PHP_CodeSniffer_Exception: Referenced sniff ../TYPO3/Sniffs/Files/OneClassPerFileSniff.php does not exist in /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php on line 852

Call Stack:
   0.0148     633800   1. {main}() /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs:0
   0.0643    2277488   2. PHP_CodeSniffer_CLI->process() /Users/andygrunwald/Development/PHP_CodeSniffer/scripts/phpcs:37
   0.0649    2285200   3. PHP_CodeSniffer->process() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer/CLI.php:545
   0.0649    2286840   4. PHP_CodeSniffer->setTokenListeners() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:448
   0.0888    2287976   5. PHP_CodeSniffer->getSniffFiles() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:643
   0.0901    2296152   6. PHP_CodeSniffer->_expandRulesetReference() /Users/andygrunwald/Development/PHP_CodeSniffer/CodeSniffer.php:750

My solution was to change the realpath-line in your commit (e329880) to

$realpath = realpath(self::$standardDir.'/'.$sniff);

Just remove the "dirname()" call.
After this change, it just works fine :)

If you need help to reproduce my error, just drop me a line and i will setup a demo.
Or is it my fault and only a small miss configuration?


Reply to this email directly or view it on GitHub:
#25 (comment)

Greg Sherwood
Product Development Manager
gsherwood@squiz.com.au

Squiz Labs Pty. Ltd. Suite 4, 7 Parkes St Parramatta NSW 2150
P  +61 2 9045 2800   W  www.squizlabs.com

@andygrunwald
Copy link
Contributor Author

Hey Greg,

thx for your work. The new commit (40d804d) works perfect!
So my pull request is obsolete, because you creates a better solution.
THX for this and now i will wait for the new release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants