Bug 461410

Comment 1 Michael Rennie 2015-03-04 11:43:17 EST

(In reply to Mark Macdonald from comment #0)
> This code currently produces no warnings:
> 
>  (function foo() {
>      return 1;
>  }());
> 
> The linter should flag the unused identifier "foo".

I'm not sure there us anything to mark here (in this particular case), since the function expression is actually being used (just not explicitly by name):

Heres the AST:

{
    "type": "Program",
    "body": [
        {
            "type": "ExpressionStatement",
            "expression": {
                "type": "CallExpression",
                "callee": {
                    "type": "FunctionExpression",
                    "id": {
                        "type": "Identifier",
                        "name": "foo"
                    },
                    "params": [],
                    "defaults": [],
                    "body": {
                        "type": "BlockStatement",
                        "body": [
                            {
                                "type": "ReturnStatement",
                                "argument": {
                                    "type": "Literal",
                                    "value": 1,
                                    "raw": "1"
                                }
                            }
                        ]
                    },
                    "rest": null,
                    "generator": false,
                    "expression": false
                },
                "arguments": []
            }
        }
    ]
}

The above is a side-effect of the parenthesis.

Mark noted you could simply change it to:

var thing = {
   method: function foo() {
   }
};

But in that case I think it makes sense to mark 'method' as not being used, and have another linting problem to mention the id of the func expression does not match the named member it is assigned to. In the case of the member and id matching, then both would be marked as not used.

Comment 2 Mark Macdonald 2015-03-04 12:19:56 EST

> In the case of the member and id matching, then both would be marked as not used.

Maybe I misunderstand what you meant, but... we have to be careful here. All we can check right now is lexical scope, which doesn't include members. For example:

 var thing = {
    foo: function foo() {
    }
 };
 thing.toString(); // use thing somehow

^ Here, we can flag the identifier appearing after the function keyword as unused (if we decide to), but we can't correctly say that the member "thing.foo" is unused. Not without doing a much deeper analysis, anyway.

Comment 3 Mark Macdonald 2015-03-04 12:24:12 EST

Mike pointed out that we might create confusion if we start warning on the code shown in Comment 0, and I agree. It's easy for a reader to mistake FunctionExpression-with-identifier for FunctionDeclaration, especially if misleading punctuation is used.

If the 2 cases were to produce similar warnings, the reader might think it's OK to delete a FunctionExpression when it's not.

So I am OK with leaving this alone.

Comment 4 Michael Rennie 2015-05-08 10:34:38 EDT

Closin wont fix. 

I think we all agree that we should try not to confuse our users :)