JSLint browser: true is enabled more often than expected

[core-ai-bot]

Issue by peterflynn
Tuesday Aug 05, 2014 at 06:53 GMT
Originally opened as adobe/brackets#8657

---

Sprint 37 (834c4e9d) added a feature that forces the JSLint browser: true option on if no other environment setting was specified. However, it only looks at project-level preferences and ignores the JSLint directive in the file being linted:

1. Make a new project (or open something without any special prefs, like Getting Started)

2. Create a new JS file with this content:

   /*jslint node: true */
   var foo = window.location;
   

3. Let JSLint run

Result: No JSLint errors

Expected: One JSLint error: "'window was used before it was defined"

Workaround: create a .brackets.json file in the root of the project with "jslint.options": { "node": true } in it.

[core-ai-bot]

Comment by peterflynn
Tuesday Aug 05, 2014 at 06:58 GMT

---

Related: we need to put browser: false in the Brackets source project's own .brackets.json -- we intentionally don't want any environment assumed (other than the devel: true that we already specify). The past few months we've been unintentionally running with browser: true due to this behavior change, so we may have a few JSLint issues to clean up as a result...

We haven't put this option explicitly in every JS file (normally it's the default in JSLint, so we had no need before), so even with this bug fixed we'll still want browser: false set in the project-wide prefs...

[core-ai-bot]

Comment by peterflynn
Tuesday Aug 05, 2014 at 07:32 GMT

---

I put up PR #8658 to address the comment above. The bug itself remains.

[core-ai-bot]

Comment by dangoor
Monday Aug 11, 2014 at 19:09 GMT

---

@peterflynn I think the issue here is that browser: true and node: true are not mutually exclusive, so JSLint is just doing both. We could scan through the file for node:\s*true or something like that and then switch off the browser flag. Does that seem reasonable?

I still agree with the original rationale that Brackets is used primarily for browser projects and it's better to default to browser: true.

[core-ai-bot]

Comment by peterflynn
Monday Aug 11, 2014 at 19:38 GMT

---

The issue for me is the inconsistency: if you add node: true to your project settings, the automatic browser: true is turned off; but if you add node: true to an individual file's settings, browser: true remains turned on. That was unexpected. I think the suggested fix seems reasonable.

[core-ai-bot]

Comment by dangoor
Monday Aug 11, 2014 at 20:00 GMT

---

Ahh, yeah. That's the problem with object values for preferences (with the current implementation). They are a strict replacement. jslint.options in the project scope completely replaces jslint.options at the user or default scopes. There's no merging that occurs.

Settings within a file's /* jslint comments are completely handled by JSLint (and can actually be scoped to functions in addition to the whole file).

[core-ai-bot]

Comment by peterflynn
Monday Aug 11, 2014 at 22:21 GMT

---

@dangoor I don't think project vs. global scope is the issue in this case: in the repro steps, there's no global setting involved. The prefs-computed jslint.options object is merged by JSLint with the options set inside the JS code itself, and that is a fine-grained merge. The problem is that our JSLint extension only looks at the prefs object when deciding whether to force browser: true to on -- it doesn't look at the latter options (the settings in the code).

I think in your earlier comment you proposed having our JSLint extension sniff the text of the document looking for node:\s*true -- that seems like it should fix the issue.

[core-ai-bot]

Comment by dangoor
Tuesday Aug 12, 2014 at 12:32 GMT

---

@peterflynn You're right! I thought it was setting {browser:true} as the default value for jslint.options, but that's not the case. I didn't remember that it was already checking for an environment flag in the options.

Yeah, the fix is definitely scanning the text for node:\s*true. It might be more complete to scan for any of the ENVIRONMENTS, but that's probably not important in practice...