Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jshint considers user delete() function as keyword, not function, yields 'function not used' for argument #1881

Closed
altmind opened this issue Sep 30, 2014 · 5 comments · Fixed by #1992
Labels

Comments

@altmind
Copy link

altmind commented Sep 30, 2014

Consider this code(you can try validate it on jshint.com)

var contactsRouter = {
  'route': function(){return this;},
  'delete': function(){return this;}
};
contactsRouter
        .route('contacts/:contact_id')
        .delete(deleteContact);

function deleteContact() {

}

JSHint yields "deleteContact" is defined, but never used.
This is wrong, deleteContact is actually used in delete().
Possibly, jshint assumes contactsRouter.delete() is keyword 'delete', not user function.
Possibly related: #1562

@altmind altmind changed the title jshint considers user delete() function as keyword, not function, yields 'function not used' jshint considers user delete() function as keyword, not function, yields 'function not used' for argument Sep 30, 2014
@caitp
Copy link
Member

caitp commented Sep 30, 2014

REPL says:

CONFIGURE
Metrics

There are 3 functions in this file.
Function with the largest signature take 0 arguments, while the median is 0.
Largest function has 1 statements in it, while the median is 1.
The most complex function has a cyclomatic complexity value of 1 while the median is 1.

One unused variable
9   deleteContact

@rwaldron
Copy link
Member

Changing delete to eg. d eliminates the warning, so there is definitely a bug in how "delete" is being treated.

@rwaldron rwaldron added the P2 label Sep 30, 2014
@jugglinmike
Copy link
Member

The problem is a little more subtle than the bug report suggests. For example:

/* jshint unused: true */
/* jshint unused: true */
var a = {};

function before() {}

a.delete(before);
a.delete(after);

function after() {}

Results in:

One warning
9   'after' is defined but never used.

Given that there is no warning issued for the variable before above, this bug must somehow be related to hoisting. Also, this behavior extends one (but not all!) of the other keyword unary operators--typeof:

/* jshint unused: true */
var a = {};

function deleteBefore() {}
function typeofBefore() {}
function voidBefore() {}
function newBefore() {}

a.delete(deleteBefore);
a.typeof(typeofBefore);
a.void(voidBefore);
a.new(newBefore);

a.delete(deleteAfter);
a.typeof(typeofAfter);
a.void(voidAfter);
a.new(newAfter);

function deleteAfter() {}
function typeofAfter() {}
function voidAfter() {}
function newAfter() {}

issues:

Two warnings
19  'deleteAfter' is defined but never used.
20  'typeofAfter' is defined but never used.

So this is going to be a fun one.

@jugglinmike
Copy link
Member

...I guess it's not surprising that new doesn't exhibit the bug, but you would think that void would trigger the same behavior.

@jugglinmike
Copy link
Member

I have a solution for this, but it is dependent on gh-1903.

jugglinmike added a commit to jugglinmike/jshint that referenced this issue Nov 20, 2014
jugglinmike added a commit to jugglinmike/jshint that referenced this issue Nov 20, 2014
Operators `typeof` and `delete` both accept null references, so JSHint
should not warn when their sole operand is a undefined variable, nor
should that identifier be included in the `implieds` array of the
generated report.

A runtime error will occur if the operand is an expression containing
the undefined identifier, so a warning should be issued in this case.

This patch also addresses an existing bug where property identifiers
with values "typeof" or "delete" were incorrectly interpreted as unary
operators when marking variables as unused.

Resolves jshintgh-1881
jugglinmike added a commit to jugglinmike/jshint that referenced this issue Nov 24, 2014
Operators `typeof` and `delete` both accept null references, so JSHint
should not warn when their sole operand is a undefined variable, nor
should that identifier be included in the `implieds` array of the
generated report.

A runtime error will occur if the operand is an expression containing
the undefined identifier, so a warning should be issued in this case.

This patch also addresses an existing bug where property identifiers
with values "typeof" or "delete" were incorrectly interpreted as unary
operators when marking variables as unused.

Resolves jshintgh-1881
jugglinmike added a commit to jugglinmike/jshint that referenced this issue Dec 20, 2014
Operators `typeof` and `delete` both accept null references, so JSHint
should not warn when their sole operand is a undefined variable, nor
should that identifier be included in the `implieds` array of the
generated report.

A runtime error will occur if the operand is an expression containing
the undefined identifier, so a warning should be issued in this case.

This patch also addresses an existing bug where property identifiers
with values "typeof" or "delete" were incorrectly interpreted as unary
operators when marking variables as unused.

Resolves jshintgh-1881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants