Skip to content
This repository has been archived by the owner on Jul 20, 2018. It is now read-only.

Use PHP Parser for checks #218

Merged
merged 19 commits into from
Apr 2, 2015
Merged

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Dec 2, 2014

This is another preview PR related to #195. This one is currently based on the other PHP-Parser PR, #204, so what's interesting is the diff between them.

Update 2014-12-20: List of checks mentioned in config-vip-scanner.php, with converted ones checked (modulo some slight general refactoring of CodeCheckBase) Note that some checks are for HTML, CSS or JavaScript and cannot be migrated to PHP-Parser.

  • UndefinedFunctionCheck
  • AtPackageCheck
  • BodyClassCheck
  • CDNCheck
  • CommentsCheck
  • CustomizerCheck
  • DeprecatedConstantsCheck
  • DeprecatedFunctionsCheck
  • DeprecatedParametersCheck
  • EscapingCheck
  • FirstPostPromptCheck
  • ForbiddenConstantsCheck
  • ForbiddenFunctionsCheck
  • ForbiddenGoogleCheck
  • ForbiddenLibrariesCheck
  • ForbiddenPHPFunctionsCheck
  • HeaderCheck
  • HooksCheck
  • InternationalizedStringCheck
  • jQueryCheck
  • JetpackCheck
  • LanguagePacksCheck
  • MasonryCheck
  • PostThumbnailsCheck
  • ScreenshotCheck
  • ThemecolorsCheck
  • ThemeNameCheck
  • ThemePostPaginationCheck
  • TitleCheck
  • VCMergeConflictCheck
  • WidgetsCheck
  • JavaScriptLintCheck
  • VIPWhitelistCheck
  • VIPRestrictedClassesCheck
  • VIPRestrictedPatternsCheck
  • VIPRestrictedCommandsCheck
  • VIPInitCheck
  • VIPParametersCheck
  • PHPShortTagsCheck
  • PHPClosingTagsCheck
  • VCMergeConflictCheck
  • WordPressCodingStandardsCheck
  • ClamAVCheck
  • AdBustersCheck

@ockham ockham mentioned this pull request Dec 3, 2014
@ockham ockham force-pushed the php-parser-checks branch 3 times, most recently from ed41afc to ae894c1 Compare December 6, 2014 12:05
@nb nb changed the title Php parser checks Use PHP Parser for checks Dec 8, 2014
@ockham
Copy link
Contributor Author

ockham commented Dec 10, 2014

I'm going to redo this branch a bit:

  • Tests are done better in Php check test base #219
  • Having experimented with migrating a couple more checks, I think that the current approach for checks and visitors would cause a load of visitors to be introduced (FunctionCallCheckVisitor and ClassCheckVisitor were likely only the tip of the iceberg), with only minimal differences. I think this is better done by passing callbacks in a column of the $checks constructor argument to a rather basic check visitor.

@ockham
Copy link
Contributor Author

ockham commented Dec 10, 2014

Illustrative example: https://gist.github.com/ockham/bffdf72507facf56c328
eval() isn't an instanceof PhpParser\Node\Expr\FuncCall, but of PhpParser\Node\Expr\Eval_. So we would otherwise need both a FunctionCallCheckVisitor and a EvalCheckVisitor here. I prefer the callbacks approach, which is IMO pretty expressive.

@ockham
Copy link
Contributor Author

ockham commented Dec 11, 2014

Based on latest experiments, this is proably going toward just having one individual visitor for each check after all, without inheriting from intermediate classes like FunctionCallCheckVisitor or EvalCheckVisitor...

@ockham ockham force-pushed the php-parser-checks branch 7 times, most recently from 5c1476c to 84579d1 Compare December 21, 2014 13:56
@ockham ockham mentioned this pull request Dec 21, 2014
@ockham ockham force-pushed the php-parser-checks branch 2 times, most recently from 167bf1c to abe33d1 Compare December 22, 2014 10:47
@ockham ockham force-pushed the php-parser-checks branch 4 times, most recently from d4dd690 to b05879d Compare February 16, 2015 19:28
@ockham ockham mentioned this pull request Feb 16, 2015
@ockham
Copy link
Contributor Author

ockham commented Mar 7, 2015

@stevegrunwell I thought you might be interested in this PR as it's supposed to tighten detection of forbidden patterns vs false positives by using an actual PHP parser instead of fiddling with regexes. Feel free to leave your comments; also see my other (related, leaner) PRs.

@ockham ockham force-pushed the php-parser-checks branch 6 times, most recently from 9b5f2ee to 0bcaee5 Compare March 31, 2015 16:58
@ockham ockham force-pushed the php-parser-checks branch 2 times, most recently from c5a6b1c to 387c795 Compare March 31, 2015 17:03
@ockham
Copy link
Contributor Author

ockham commented Mar 31, 2015

Moved logic from CodeCheckVisitor to CodeCheck as previously announced. Should be good to merge now.

@ockham
Copy link
Contributor Author

ockham commented Mar 31, 2015

Hrm, tests failing with PHP 5.3. I'll fix that as soon as I can.

sboisvert added a commit that referenced this pull request Mar 31, 2015
Converts Analyzer to PHP parser.
Lays the groundwork for converting tests to PHP-Parser (See #218 )

Automattic/vip-quickstart@8c48dc3
Should make it so that Quickstart works with the sub modules this needs.
@ockham ockham force-pushed the php-parser-checks branch from 387c795 to 7ef01b2 Compare March 31, 2015 18:53
@ockham ockham force-pushed the php-parser-checks branch from d655001 to 24db3a1 Compare April 2, 2015 17:28
ockham added a commit that referenced this pull request Apr 2, 2015
@ockham ockham merged commit cbd9448 into Automattic:master Apr 2, 2015
@ockham ockham deleted the php-parser-checks branch April 2, 2015 17:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant