Faulty check for angular/function-type

[taa-autorola-com]

The fule angular/function-type fails when using specific function names and argument patterns. More specific, the reserved names are defined in the rule as var angularObjectList = ['animation', 'config', 'constant', 'controller', 'directive', 'factory', 'filter', 'provider', 'service', 'value', 'decorator'];

When any of those predefined functions are seen and the parameters match the rule specification , the rule is enforced.

I've created a minimal example here, which shows a function named filter, which should probably not be checked by the rule.

`(function () { 'use strict';

angular
    .module('bugModule')
    .controller('BugController', BugController);

BugController.$inject = [];

/* @ngInject */
function BugController() {
    /* jshint validthis:true */
    var vm = this;

    var _ = {
        filter: function (arr, fn) {
            var res = [];
            for (var i = 0; i < arr.length; ++i) {
                if (fn(arr[i])) {
                    res.push(arr[i]);
                }
            }
            return res;
        }
    };
    vm.global = [1, 2, 3, 4];

    activate();

    // -------------------------------------------------------------------------

    function activate() {
        // global vm variable: OK
        _.filter(vm.global, function (n) { return n % 2 === 0; });

        // function: OK
        _.filter(getGlobal(), function (n) { return n % 2 === 0; });

        // inlined: OK
        _.filter([1, 2, 3, 4], function (n) { return n % 2 === 0; });

        // local: FAIL
        var local = [1, 2, 3, 4];
        _.filter(local, function (n) { return n % 2 === 0; });

        doFilter(local);
    }

    function doFilter(arr) {
        // parameter: same as local: FAIL
        return _.filter(arr, function (n) { return n % 2 === 0; });
    }

    function getGlobal() {
        return vm.global;
    }
}

})(); `

It seems it is the argument checking that fails in this case, since it works when using functions or global variables. Only local variables (and as function parameters) are affected.

[taa-autorola-com]

Oops, forgot to mention the versions: eslint: 3.3.1 angular: 1.4.8 eslint-plugin-angular: 1.6.2

[ProdigySim]

I started seeing a similar issue to this today with one-dependency-per-line, right after I cleared my node_modules folder and reinstalled. ~~~Upstream dependency issues?~~~ Nevermind, just noticed my eslint-plugin angular updated itself to 1.6.2

[mateatslc]

_.filter is failing all over the place for me too

[Autalyst]

I am also having an issue with underscore's _.filter function getting caught in this.

[EmmanuelDemey]

I have just pushed a unit test, but I was unable to reproduce the problem. Can you update my unit test ?

[Autalyst]

I am unable to reproduce the issue anymore.

[calling]

I'm still seeing this issue occur: eslint: 4.1.1 eslint-plugin-angular: 2.4.2 angular: 1.5.11

Rule:

"angular/function-type": ["error", "named"]

Failures are with _.filter.

The unit test added looks like it's for anonymous, not for named - could that be why it's not reproducing the issue?

[taa-autorola-com]

Just tested this and I can confirm the issue is still there... The code in the original post still fails.

[taa-autorola-com]

"angular/function-type": [2, "named"] causes the error to be reported, incorrectly.

"angular/function-type": [2, "anonymous"] or the equivalent "angular/function-type": 2 does not report any errors.

So I'm pretty sure @calling is on to something.

[jfgreffier]

I've added test case that reproduces the issue described by @taa-autorola-com

valid.push({
    code: '_.filter(vm.global, function (n) { return n % 2 === 0; })',
    options: ['anonymous']
}, {
    code: '_.filter(vm.global, function (n) { return n % 2 === 0; })',
    options: ['named']
}, {
    code: '_.filter(local, function (n) { return n % 2 === 0; })',
    options: ['named']
}, {

First two cases pass, the third one (local variable) fails

[EmmanuelDemey]

@jfgreffier have you sent a PR ?

[jfgreffier]

No, I just did a fork with the unit test. I don't have fix to put in in a pull request

Here's the commit with the test: https://github.com/jfgreffier/eslint-plugin-angular/commit/95ee8d30323225262e7821ca673cb2f21f25b3ed

It appears in this conversation since it reference this bug. Do you want me to do a PR with this unit test ?

[EmmanuelDemey]

It would be great :) thanks a lot.

[jfgreffier]

Done ;)

It seems to be linked with utils.isAngularComponent() yielding a false positive; as it does indeed look like Angular code...

[EmmanuelDemey]

I think the problem come from this snippet

function isNamedInlineFunction(node) {
    console.log(this.isFunctionType(node), node.id, node)
    return this.isFunctionType(node) && node.id && node.id.name && node.id.name.length > 0;
}

The node.id looks to be null. I do not know why. still looking at this issue.

[jfgreffier]

Here is a pull request with a proposed fix.

This code fits the conditions of isAngularComponent()

_.filter(local, function (n) { return n % 2 === 0; })

Callee can be anything
First argument can be a literal (String) or an identifier (variable)
Second argument is often a function, named or not.
-> "They're The Same Picture"

My solution is to explicitly rule out callee from a reserved name list.

var myModule = angular.module("foo", []);

// The rule applies
myModule.filter(local, function (n) { return n % 2 === 0; })

// Excluded from the rule
_.filter(local, function (n) { return n % 2 === 0; })