squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Version 2.3.2: PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php #635

Closed mathieuchateau closed 9 years ago

mathieuchateau commented 9 years ago

Hello,

running phpcs against a folder of php site, I have a huge number of this: PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3476 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3464 (always same 2 lines in loop) on Centos 7.1, latest yum update

command line: phpcs -d memory_limit=512M /path/to/folder nothing changhed in phpcs, just installed it 10mn ago

mathieuchateau commented 9 years ago

another one, the first before the others: Notice: Undefined index: */ in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718

aik099 commented 9 years ago

Please provide minimal code sample that would cause the notice to appear. I guess it's a JavaScript file.

mathieuchateau commented 9 years ago

It's a big folder (a whole application). I will try to filter doing subfolder per subfolder to isolate

aik099 commented 9 years ago

You can add -v (like verbose) flag to phpcs command to see which file is being processed at the moment when notice is triggered.

mathieuchateau commented 9 years ago

-v is helpful, indeed

For first error:

Processing jquery-ui-1.10.3.custom.js PHP Notice: Undefined index: / in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718 PHP Notice: Undefined index: / in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718 PHP Notice: Undefined index: */ in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718

aik099 commented 9 years ago

The jquery-ui-1.10.3.custom.js file is non-minified?

mathieuchateau commented 9 years ago

For the second one: Processing jsoneditor.js [JS => 64155 tokens in 6926 lines]... PHP Notice: Undefined index: scope_condition in /usr/share/pear/PHP/CodeSniffer/Standards/Generic/Sniffs/WhiteSpace/ScopeIndentSniff.php on line 417 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3464 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3464 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3476 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3464 PHP Notice: Undefined index: in /usr/share/pear/PHP/CodeSniffer/File.php on line 3464

mathieuchateau commented 9 years ago

yes jquery-ui-1.10.3.custom.js is the non minified one. Same folder have the same file with .min. which is minified

aik099 commented 9 years ago

Could you please create a gist with contents of mentioned files (ones, that you scan for errors) as well?

mathieuchateau commented 9 years ago

a gist ? what's that?

mathieuchateau commented 9 years ago

If doing only this file then behavior is better

Processing jquery-ui-1.10.3.custom.js PHP Notice: Undefined index: / in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718 PHP Notice: Undefined index: / in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718 PHP Notice: Undefined index: */ in /usr/share/pear/PHP/CodeSniffer/Tokenizers/JS.php on line 718 [JS => 151167 tokens in 14972 lines]... DONE in 24.65 secs (37656 errors, 0 warnings)

FILE: .../my/path/to/jquery-ui-1.10.3.custom.js

FOUND 37656 ERRORS AFFECTING 12687 LINES

 8 | ERROR | [x] Line indented incorrectly; expected at least 4
   |       |     spaces, found 0
 9 | ERROR | [x] Spaces must be used to indent lines; tabs are
   |       |     not allowed
 9 | ERROR | [x] Line indented incorrectly; expected at least 4
   |       |     spaces, found 1
11 | ERROR | [x] Line indented incorrectly; expected at least 4
   |       |     spaces, found 0
12 | ERROR | [x] Line indented incorrectly; expected at least 4
   |       |     spaces, found 0
14 | ERROR | [x] Line indented incorrectly; expected at least 4
   |       |     spaces, found 0
14 | ERROR | [x] Opening parenthesis of a multi-line function
   |       |     call must be the last content on the line
aik099 commented 9 years ago

a gist ? what's that?

It is this: https://gist.github.com/ . It's preferred to attach file content in there (one gist even can contain multiple files), rather then copy-pasting large files as issue/pr comments.

mathieuchateau commented 9 years ago

json-editor is a public json:

/*! JSON Editor v0.6.19 - JSON Schema -> HTML Editor

source code for this version: https://github.com/jdorn/json-editor/archive/v0.6.19.zip

aik099 commented 9 years ago

Why you're checking errors in vendor libraries, that you have no control over at all?

mathieuchateau commented 9 years ago

I am not the developpers, didn't know site was including third party library. Just trying to gather all code issues for site.

I can try to exclude this library, but I think you should still fix this issue, as I may have same code generating same sort of error in internal code.

aik099 commented 9 years ago

Sure. The reported issue is absolutely valid. I'm just saying that it's better to exclude vendors from checks.

mathieuchateau commented 9 years ago

I agree too

gsherwood commented 9 years ago

The only file that caused issues when I checked the repo you linked was this one: https://github.com/jdorn/json-editor/blob/master/src/intro.js

In isolation, which is how PHPCS treats each file, that isn't valid JS code. I don't really want to add a bunch of checks to support code like that, especially if this is in a library that you don't need to be checking anyway.

If this is your library, I'd suggest ignoring intro.js and outro.js as these are not valid JS files and don't need to be checked.

Do you have links to any valid JS files that are causing tokenizer errors?

mathieuchateau commented 9 years ago

Hello,

did you try using my version and not the current one (the zip url I provided) ? I have this issue with json-editor file , at least in the version provided

gsherwood commented 9 years ago

did you try using my version and not the current one (the zip url I provided) ?

I downloaded the ZIP you linked to, extracted it, and ran the Squiz standard (the only included standard that checks JS files) over the whole thing.

Do you have a custom standard I need to use?

mathieuchateau commented 9 years ago

I am bit lost. I downloaded also this zip file and I have same issue when starting phpcs against it..

I have this version: PHP_CodeSniffer version 2.3.2 (stable) by Squiz (http://www.squiz.net)

mathieuchateau commented 9 years ago

I just did a yum install of phpcs,, without changing anything

gsherwood commented 9 years ago

Sorry, I was using the latest stable version. 2.3.2 does show those errors, but 2.3.3 does not. Please try updating and see if you still have a problem.

mathieuchateau commented 9 years ago

I don't have more recent one from yum repo. I had this version from this one: Installed Packages Name : php-pear-PHP-CodeSniffer Arch : noarch Version : 2.3.2 Release : 1.el7 Size : 3.1 M Repo : installed From repo : epel Summary : PHP coding standards enforcement tool URL : http://pear.php.net/package/PHP_CodeSniffer License : BSD Description : PHP_CodeSniffer provides functionality to verify that code conforms to : certain standards, such as PEAR, or user-defined.

gsherwood commented 9 years ago

The only methods I maintain for installing PHP_CodeSniffer are PEAR, Composer and the Phar files. I don't have anything to do with that yum package, but it's obviously out of date now and there isn't anything I can do about that.

Maybe try using PEAR directly? pear upgrade php_codesniffer

mathieuchateau commented 9 years ago

Understood. I thought you was validating version to push through repos. epel is the official one for Fedora Project.

I did the upgrade with pear and now it's much better :)

I still have some PHP notice, but I guess we can now close this case as it's not looping forever

Thanks for your support

gsherwood commented 9 years ago

I still have some PHP notice, but I guess we can now close this case as it's not looping forever

Those are probably the same noticed I mentioned above, which are when checking that invalid JS file, If not, please report a new issue and I'll take a look. Thanks for testing the new version.

kenorb commented 8 years ago

Hundreds of these notices I've seen when checking not-minified jquery-ui.js file (jQuery UI - v1.11.4 - 2015-03-11)

Fixing file: 30861/30861 violations remaining PHP Notice: Undefined index: / in ~/.composer/vendor/squizlabs/php_codesniffer/CodeSniffer/Tokenizers/JS.php on line 718 PHP Notice: Undefined index: / in ~/.composer/vendor/squizlabs/php_codesniffer/CodeSniffer/Tokenizers/JS.php on line 718

I don't if it's looping (it keeps backing to 0) and it's taking forever. I'm on 2.0.x-dev.

aik099 commented 8 years ago

Is it a minified file? Also why you're checking your vendors in the first place.

kenorb commented 8 years ago

@aik099 Not minified. Human-readable code. Scanned it just by mistake with other files which were included in the current folder (as I've included js extension) and left it for some while. Not a big issue, but I was curious if there is something reported already, so I'm sharing in case there is something to fix.

aik099 commented 8 years ago

What exact version of PHP_CodeSniffer are you using?

Try using 2.3.4 and master branch to see if problem is solved in there.

gsherwood commented 8 years ago

@kenorb I think I figured this out. It was a bad regular expression detection happening in the JS tokenizer. It saw part of a comment as a regular expression. Fixed now.

kenorb commented 8 years ago

I've the same notices again + endless loop, this time with PHP code, but I think it's related to Drupal Sniff, so reported the issue there.