2013-05-19 02:04:22 +02:00
|
|
|
{
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
"bitwise": true,
|
|
|
|
|
"curly": true,
|
2015-01-20 11:25:12 +01:00
|
|
|
"freeze": true,
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
"immed": true,
|
|
|
|
|
"indent": 2,
|
Disable the JSHint "function called before it is defined" and "unused parameter" warnings
Summary:
Ref T12822. The next change hits these warnings but I think neither is a net positive.
The "function called before it is defined" error alerts on this kind of thing:
```
function a() {
b();
}
function b() {
}
a();
```
Here, `b()` is called before it is defined. This code, as written, is completely safe. Although it's possible that this kind of construct may be unsafe, I think the number of programs where there's unsafe behavior here AND the whole thing doesn't immediately break when you run it at all is very very small.
Complying with this warning is sometimes impossible -- at least without cheating/restructuring/abuse -- for example, if you have two functions which are mutually recursive.
Although compliance is usually possible, it forces you to define all your small utility functions at the top of a behavior. This isn't always the most logical or comprehensible order.
I think we also have some older code which did `var a = function() { ... }` to escape this, which I think is just silly/confusing.
Bascially, this is almost always a false positive and I think it makes the code worse more often than it makes it better.
---
The "unused function parameter" error warns about this:
```
function onevent(e) {
do_something();
```
We aren't using `e`, so this warning is correct. However, when the function is a callback (as here), I think it's generally good hygiene to include the callback parameters in the signature (`onresponse(response)`, `onevent(event)`, etc), even if you aren't using any/all of them. This is a useful hint to future editors that the function is a callback.
Although this //can// catch mistakes, I think this is also a situation where the number of cases where it catches a mistake and even the most cursory execution of the code doesn't catch the mistake is vanishingly small.
Test Plan: Egregiously violated both rules in the next diff. Before change: complaints. After change: no complaints.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12822
Differential Revision: https://secure.phabricator.com/D20190
2019-02-17 15:24:08 +01:00
|
|
|
"latedef": "nofunc",
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
"newcap": true,
|
|
|
|
|
"noarg": true,
|
|
|
|
|
"quotmark": "single",
|
|
|
|
|
"undef": true,
|
Disable the JSHint "function called before it is defined" and "unused parameter" warnings
Summary:
Ref T12822. The next change hits these warnings but I think neither is a net positive.
The "function called before it is defined" error alerts on this kind of thing:
```
function a() {
b();
}
function b() {
}
a();
```
Here, `b()` is called before it is defined. This code, as written, is completely safe. Although it's possible that this kind of construct may be unsafe, I think the number of programs where there's unsafe behavior here AND the whole thing doesn't immediately break when you run it at all is very very small.
Complying with this warning is sometimes impossible -- at least without cheating/restructuring/abuse -- for example, if you have two functions which are mutually recursive.
Although compliance is usually possible, it forces you to define all your small utility functions at the top of a behavior. This isn't always the most logical or comprehensible order.
I think we also have some older code which did `var a = function() { ... }` to escape this, which I think is just silly/confusing.
Bascially, this is almost always a false positive and I think it makes the code worse more often than it makes it better.
---
The "unused function parameter" error warns about this:
```
function onevent(e) {
do_something();
```
We aren't using `e`, so this warning is correct. However, when the function is a callback (as here), I think it's generally good hygiene to include the callback parameters in the signature (`onresponse(response)`, `onevent(event)`, etc), even if you aren't using any/all of them. This is a useful hint to future editors that the function is a callback.
Although this //can// catch mistakes, I think this is also a situation where the number of cases where it catches a mistake and even the most cursory execution of the code doesn't catch the mistake is vanishingly small.
Test Plan: Egregiously violated both rules in the next diff. Before change: complaints. After change: no complaints.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12822
Differential Revision: https://secure.phabricator.com/D20190
2019-02-17 15:24:08 +01:00
|
|
|
"unused": "vars",
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
|
|
|
|
|
"expr": true,
|
|
|
|
|
"loopfunc": true,
|
|
|
|
|
"sub": true,
|
|
|
|
|
|
|
|
|
|
"globals": {
|
|
|
|
|
"JX": false,
|
2016-01-28 22:29:27 +01:00
|
|
|
"d3": false,
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
"__DEV__": false
|
|
|
|
|
},
|
2016-01-28 22:29:27 +01:00
|
|
|
|
Be more strict with JSHint.
Summary: Add a bunch of extra checks to be performed by `jshint`. For documentation, see http://jshint.com/docs/options/.
Test Plan:
Ran `jshint --config support/jshint/jshintconfig webroot/rsrc/js/`. There were a bunch of existing violations, but some of these are legitimate and probably require attention.
```lang=json
{
"bitwise": true, // 0 violations
"curly": true, // 0 violations
"immed": true, // 1 violation
"indent": 2, // 0 violations
"latedef": true, // 10 violations
"newcap": true, // 1 violation
"noarg": true, // 0 violations
"quotmark": "single", // 55 violations
"undef": true, // 24 violations
"unused": true, // 107 violations
"expr": true,
"loopfunc": true,
"sub": true,
"globals": {
"JX": false,
"__DEV__": false
},
"browser": true
}
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9659
2014-06-22 20:07:52 +02:00
|
|
|
"browser": true
|
2013-05-19 02:04:22 +02:00
|
|
|
}
|
