jshint / jshint

JSHint is a tool that helps to detect errors and potential problems in your JavaScript code
http://jshint.com
MIT License
8.98k stars 1.65k forks source link

JSON always a known global, can not exclude a global in the jshintrc file #1603

Open Knotschi opened 10 years ago

Knotschi commented 10 years ago

how can i tell jshint that it should create an error when i use JSON like JSON.stringify or JSON.parse without defining it? JSON is not a valid global on old IEs. I am using json2 but because i have an amd style application that should never write in the window object, i have to require json2 in each file where i use JSON.

i tried ec3, browser: false and a lot other stuff. the only thing that worked was adding /* global -JSON */ on the top of each js file. but i would like to add a configuration for that in the .jshintrc

is there an option for the jshintrc file for disallowing a special global? if not: that would be a nice feature.

valueof commented 10 years ago

I think putting "-JSON": true into your .jshintrc should work as well. Let me know if it doesn't and I'll reopen this.

Knotschi commented 10 years ago

that didnt worked. i also tried out "JSON":false and "-JSON": false. nothing worked. the only thing that worked was adding this global -JSON to every file

rwaldron commented 10 years ago

I am using json2 but because i have an amd style application that should never write in the window object, i have to require json2 in each file where i use JSON.

Seems like this is being over-complicated. "json2" provides a shim for a global built-in, so it should be treated like a global built-in, ie. not subject to user-code amd rules.

rwaldron commented 10 years ago

Another way to think about it: require.js is always included via script, because there is no other way and because you actually want to create the require and define globals, as if they were actually platform built-ins (think about the way node.js provides a platform built-in require). The same is true for JS language-level shims like "json2".

Knotschi commented 10 years ago

no, i am using browserify and bundle all my javascript files with it. at the end i have one javascript file that is kind of a api that other pages are including. i dont want to change anything in there window object, because i could create issues on the clients pages that are including our script. thats why i dont use json2 as a shim. i dont think this is overcomplicated, but i am open for suggestions how i can improve that without changing anything in the clients window object.

JSON is an undefined object in older versions of internet explorer. it should be possible to tell jshint that it should show an error when JSON is used without defining it before.

paul-wade commented 10 years ago

so the problem is that jshint recognizes JSON as a global and there is no way to shut it off? If my understanding is correct then the question is how should we implement that for configuration? a bool option? JSONGlobal:true|false. on by default??

paul-wade commented 10 years ago

I submitted a pull request for this I added a new bool option 'nojsonglobal' and if it is on I delete JSON from predefined in the assume function.

paul-wade commented 10 years ago

I implemented this change as a bool option "nojsonglobal" which for one is a dumb name and second in hindsight this really shouldn't be a bool option.

Other globals could come up in the future that you might want to exclude? I was thinking predef could be used to opt out of globals as well? predef : ["-json", "angular"] or maybe a separate array like predef but specifically for shutting off globals.undef: ["json"]?

paul-wade commented 10 years ago

It looks like line 4610 in jshint.js properly parses predef options that start with a - and adds them to the black list. I tested this and see JSON being added to the black list as expected but it seems the problem may be that the blacklist isn't being used? or maybe because JSON is an internally defined global it is ignored... Im working on a fix

paul-wade commented 10 years ago

Got it. JSON was already added via Combine before the blacklist entry was added from the predef option and when the blacklist option was set it didn't check to see if the global already existed.

machty commented 10 years ago

Raised (and closed as dup) a similar issue in #1793 surrounding the Promise global obj.