search

WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
270 stars 3 forks source link

Feature/js syntax check #142

Closed timelsass closed 5 years ago

timelsass commented 5 years ago

As mentioned by @joyously in https://github.com/WPTRT/theme-sniffer/issues/81#issuecomment-466798094, this PR adds the syntax checking done by core in the editor windows. Unfortunately eslint isn't entirely stable to run in the browser, but at least a minimal syntax check on js is fitting for this plugin and review process.

In this PR the checks are done still using the filtering on minified files from php in the callback, and the checks happen on the callback return, so after PHPCS runs.

To test: 1 create a file called test.js and throw it in the twentyninteen theme, with the content from Esprima's validate demo. 2 run theme check on twentyninteen theme. You should get output towards the end of the report similar to this: image

Thoughts:

  1. Ideally we should setup a worker pool and handle processing for the js files outside of the main thread as they could have a potentially large amount of files.
  2. This only was made for the standard "visual" reporter, and not the raw output.
  3. I'm not sure that PHPCS needs to actually do anything with the JS files. We could offload some of the lints from PHPCS into Esprima (if there are any from https://github.com/WPTRT/WPThemeReview ). In testing Esprima is able to handle creating an AST on minfied and large files better than what we get from PHPCS crashing and/or memory issues. Since phpcs isn't running workers since there's a custom runner this could help alleviate those out of memory checks, and we wouldn't have to perform the file contents checks.
  4. Since ultimately we would probably want to get eslint browser compatible, it might be ideal to instead use https://github.com/eslint/espree - initially a fork of esprima(now built on acron), and the parser used in eslint. I figured for the time being, using the core provided functionality suffices.
dingo-d commented 5 years ago

Is there any more work to be done on this PR or can we merge it? I'm fine with letting PHPCS handle only php files, as currently there are some false positives when PHP encounters css file for instance.