enforceExistence for local functions

[wbyoung]

If I have a function that's only ever used inside of another function, I don't think it should it require documentation:

/**
 * Find capitalized words.
 */
var capitalizedWords = function(words) {
  var isCapitalized = function(string) { return string.match(/^[A-Z]/); };
  return words.filter(isCapitalized);
};

In the above example, isCapitalized would never be accessible to anyone. This doesn't seem much different from not documenting the anonymous version:

/**
 * Find capitalized words.
 */
var capitalizedWords = function(words) {
  return words.filter(function(string) { return string.match(/^[A-Z]/); });
};

Would an option for enforceExistence to let these go undocumented be a good addition? Would it be hard to add?

[qfox]

Thanks for idea.

It's not so hard to add. I'm not sure either we should add just an exception or make a new rule option. Thoughs?

[wbyoung]

@zxqfox I could see it going both ways. Different ideas:

I like this the best:

{
  "enforceExistence": {
    "exceptLocals": true,
    "exceptExports": true
  }
}

This wouldn't support all combinations of cases:

{
  "enforceExistence": "exceptLocals",
}

And this seems confusing (does it enable enforce existence or only apply when enabled):

{
  "enforceExistenceIgnoreLocals": true,
}

[wbyoung]

In the first example, the following two shorthand notations:

{
  "enforceExistence": "exceptLocals"
}

{
  "enforceExistence": "exceptExports"
}

Would be short for the following (respectively):

{
  "enforceExistence": {
    "exceptLocals": true
  }
}

{
  "enforceExistence": {
    "exceptExports": true
  }
}

[qfox]

I'm gonna to replace except* flags with allExcept array or params (to be consistent with JSCS).

So, rule should looks like:

{
  "enforceExistence": {
    "allExceptLocals": ["locals", "exports"]
  }
}

Hope this is fine ;-)

[wbyoung]

@zxqfox sounds perfect. Did you mean:

{
  "enforceExistence": {
    "allExcept": ["locals", "exports"]
  }
}

[qfox]

@wbyoung Ouh, yeah, sure! :smirk:

[dtracers]

Who do you check this for class definition? (pre es6)

function Class() {

 // needs javadocs
 this.publicFunction = function() {  helperFunction(); };

 // I would argue this should also have javadocs.
 function private helperFunction() {
   // I would argue this should not have javadocs
   // (or its documentation should be elsewhere not in the code flow)
   element.onclick = function clickFunction() {
   };
 }

}

[qfox]

@dtracers Agreed. Specs should cover them all!

[mattvot]

@zxqfox or anyone else. Is this feature currently being developed, if so do we have an ETA?

If not I'll make an attempt to develop it.

[qfox]

@mattvot Give it a try!

[mattvot]

@zxqfox I'm currently attempting to refactor the enforceExistence rule to accommodate more options.

Can you take a look at the progress so far: https://github.com/mattvot/jscs-jsdoc/blob/edca9079bc171502dda09c962d9e1a9af5426371/lib/rules/validate-jsdoc/enforce-existence.js

I'm having difficulty understanding how to define types of functions (anonymous, expressions etc). Do you have any advice regarding this?

[qfox]

I'm having difficulty understanding how to define types of functions (anonymous, expressions etc). Do you have any advice regarding this?

@mattvot That's how we usually checking function context:

        file.iterateNodesByType(['FunctionExpression'], function(node) {
            var parent = node.parentNode;
            if (parent && (
                // exception for object shorthand methods
                parent.method === true ||
                // exception for getter
                parent.kind === 'get' ||
                // exception for setter
                parent.kind === 'set' ||
                // exception for class methods
                parent.type === 'MethodDefinition' ||
                // don't error due to possible use of 'this' #1413
                parent.type === 'AssignmentExpression' ||
                // don't error due to possible use of 'this' #1413
                parent.type === 'Property')
            )

Anonymous are functions without name usually.

Can you take a look at the progress so far:

I'd like to move options parsing to configure. E.g. like it does in https://github.com/mattvot/jscs-jsdoc/blob/edca9079bc171502dda09c962d9e1a9af5426371/lib/rules/validate-jsdoc/check-types.js or (if you need more customized) https://github.com/mattvot/jscs-jsdoc/blob/edca9079bc171502dda09c962d9e1a9af5426371/lib/rules/validate-jsdoc/check-annotations.js#L17

Hope it helps! ;-)

[mattvot]

Thanks, it helped.

I've added a few more tests, refactored the configuration as you suggested and figured out some of the specifics required.

So here's the latest progress of the rule https://github.com/mattvot/jscs-jsdoc/blob/82598e884fc084861bd460b880a443426531ae73/lib/rules/validate-jsdoc/enforce-existence.js

and the test https://github.com/mattvot/jscs-jsdoc/blob/82598e884fc084861bd460b880a443426531ae73/test/lib/rules/validate-jsdoc/enforce-existence.js

Currently the test suite passes.

You can see at the top of the test file I have commented out the configuration tests. Are you able to help me figure out how to test configuration?

[qfox]

@mattvot Nice one! I've left some notes here: https://github.com/mattvot/jscs-jsdoc/commit/82598e884fc084861bd460b880a443426531ae73

Do you mind about creating PR ? It looks like almost ready :smirk_cat:

[mattvot]

Great. I'll be working on your comments sometime in the next few days and then issue a PR.

[mattvot]

FYI, still intending on PRing my changes, not had the time yet. Next week maybe.

[mattvot]

PR issued, #126