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

Detect conditional code #160

Closed
llaville opened this issue Jan 14, 2015 · 10 comments
Closed

Detect conditional code #160

llaville opened this issue Jan 14, 2015 · 10 comments
Assignees
Milestone

Comments

@llaville
Copy link
Owner

Some issues #128, #155, #159 were reported about conditional code.
We are in front of a challenge, because PHP-Parser 1.0.2 is not yet able to check pre-conditions (if code found) before traversing the AST.

A study case to understand what I means is the issue #155, where results depend on lexical order of fallback implementation.

I'll implement here the solution proposed in PHP-Parser

@llaville llaville self-assigned this Jan 14, 2015
@llaville llaville added this to the 4.0.0 milestone Jan 14, 2015
@llaville llaville mentioned this issue Jan 14, 2015
30 tasks
@llaville
Copy link
Owner Author

Node processor were implemented in Reflect with commit llaville/php-reflect@c64e412

@llaville
Copy link
Owner Author

Depending on parsing file order, some code conditions are not detected

@llaville llaville reopened this Jan 22, 2015
@llaville
Copy link
Owner Author

Finally, I've found a compromise solution. A little loss of performance, but a real result.

@llaville
Copy link
Owner Author

Files to process need to be reordered. See commit llaville/php-reflect@a468e37

@llaville
Copy link
Owner Author

Even, if we have solved challenge to identify conditional code on multiple files, and in different lexical order, we can improved again the detection.

See PHP_CodeSniffer 2.2 is a very good study case !

@llaville
Copy link
Owner Author

To solve detection of conditional code in multiple files, I've first (#160 (comment)) implemented a queue to sort file by priority (cond code first).

But, always with PHP Code Sniffer 2.2, I found it's not enough especially when using the reg exp and Symfony Finder filter iterator.

We can found in such condition, term (like "defined") in a string. That did not tell us that we are in face of conditional code with php defined function.

Example:
PHP_CodeSniffer-2.2.0\CodeSniffer\Standards\PSR2\Sniffs\Classes\ClassDeclarationSniff.php Line 80 and 82

I'll then apply a new strategy, replacing reg exp by php tokens

@llaville
Copy link
Owner Author

Commit llaville/php-reflect@ba1df8f is not enough to solve the situation.

If you don't understand, try with following snippet code

<?php

$content1 = <<<EOL
if (defined('T_TRAIT') === false) {
    define('T_TRAIT', 1105);
}
EOL;
$content2 = <<<EOL
    recordMetric('Class defined in namespace', 'no');
EOL;
$content3 = <<<EOL
    recordMetric('Class defined (in namespace)', 'no');
EOL;

$matches = null;

preg_match('/defined\s*\(/i', $content3, $matches);

var_export($matches);

$content1 and $content2 are OK, but $content3 gave a false positive with fix llaville/php-reflect@ba1df8f

@llaville
Copy link
Owner Author

confirmed now llaville/php-reflect@42eb4e9 by the additional token strategy

@llaville
Copy link
Owner Author

Not enough (: There is still another case not catched here.

When we use a conditional code before it was defined, even with file sort startegy.

For example, see PHP_CodeSniffer-2.2.0\CodeSniffer\File.php, and getDeclarationName() method that used T_TRAIT.

T_TRAIT is defined later in queue file PHP_CodeSniffer-2.2.0.tgz\PHP_CodeSniffer-2.2.0\CodeSniffer\Tokens.php.

Too late, and it generates a FALSE positive !

@llaville
Copy link
Owner Author

With llaville/php-reflect@42eb4e9...fb522bb we finally solved previous conflict issue

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

No branches or pull requests

1 participant