[[FEAT]] Support .jshintrc "extends" with no specific path.

[tomerle]

Extending the abilities of the .jshintrc file option "extends".
The new behavior resembles the root option of .editorconfig files.

When the boolean value true is used:
JSHint will walk up the directory tree and look for a .jshintrc file.

If no .jshintrc file is found (root directory reached):
Nothing will happen, and the original .jshintrc file will be used as is.

If a .jshintrc file is found:
JSHint will behave as if the file's path was explicitly passed as the "extends" option.
This includes recursive behavior for this feature.

[coveralls]

Coverage remained the same at 96.56% when pulling 2574f3e on tomerle:jshintrc-extend-autofind-parent into 7aa3e23 on jshint:master.

[jugglinmike]

Thanks for the patch! Can you talk a bit about the motivation for this feature?

[tomerle]

Most of the times I use the "extends" option or see someone else using it, the usage is pretty much the same - Counting directories up to a file (also) called .jshintrc.
I especially noticed that if a parent directory contains a .jshintrc file, it will not be skipped in favor of an "older" ancestor directory.

Taking this into consideration,
I think that having to write the path to the extended .jshintrc file (even if relative) just opens the door for unwanted human errors.
And it seems to me that the suggested feature behavior is pretty intuitive.

Use Case
Using the "extends" options in a multi environment project
Let's assume we have a Node.js project with client side code in it and tests for both:

root/
  |-.jshintrc
  |-sever.js
  |-client/
      |-.jshintrc
      |-client.js
  |-tests/
      |-.jshintrc
      |-server.spec.js
      |-client/
          |-.jshintrc
          |-client.spec.js

I believe most of the times something like this will be used:

root/.jshintrc - { "node": true ...}
root/tests/.jshintrc - { "extends": "../.jshintrc", "jasmine": true ...}
root/tests/client/.jshintrc - { "extends": "../.jshintrc", "browser": true ...}

And not something like this:

root/.jshintrc - { "node": true ...}
root/tests/.jshintrc - { "extends": "../.jshintrc", "jasmine": true ...}
root/tests/client/.jshintrc - `{ "extends": "../../.jshintrc", "jasmine": true, "browser": true ...}

So there is no reason to have to write "../.jshintrc" every time.
Also, if the suggested feature will be used, like this:

root/.jshintrc - { "node": true ...}
root/tests/.jshintrc - { "extends": true, "jasmine": true ...}
root/tests/client/.jshintrc - `{ "extends": "true", "jasmine": true, "browser": true ...}

Moving the directory root/tests to root/tests/unit will require no maintenance to the .jshintrc files.

---

I planned on writing another use case, but I feel like I wrote too much :)

[jugglinmike]

Thanks for the info!

I think that having to write the path to the extended .jshintrc file (even if relative) just opens the door for unwanted human errors.

JSHint will exit immediately if any extends option does not resolve to a readable file, so human error will be caught and reported immediately.

And it seems to me that the suggested feature behavior is pretty intuitive.

I'm not so sure about that. The value true doesn't communicate very much at all. Obviously we can't expect people to use JSHint without a little research, but explicit paths are far more intuitive/discoverable.

Moving the directory root/tests to root/tests/unit will require no maintenance to the .jshintrc files.

That's definitely true. It also makes the project's configuration a little more opaque; human readers will also have to do this recursive lookup in order to understand how JSHint will behave. And there's the performance overhead to consider--each invocation would trigger a number of I/O operations as JSHint pokes around the filesystem to find configuration files that rarely move.

As is characteristic of a lot of implicit functionality in software, there is a fair amount of subjectivity in handling exceptional cases. As the TODO comment in your patch implies, it's not clear what should happen when no file is detected. My inclination would be to fail with an error message, but I could see a case being made for proceeding silently.

I'm feeling lukewarm about this feature because I think explicitness is valuable in configuration files. As a fellow programmer, I appreciate your desire to automate this process, but I think this introduces more complexity for a rare use case--how often are users completely re-organizing their project's directory structure?

[tomerle]

Thanks for considering my suggestion.

I had some time to think about this, and I still believe this is a good behavior to add to JSHint - So, I'll try to convince you a bit more (hope that's OK). I'll concentrate on the intuitiveness since this is the reason I made the patch, and if the patch does not achieve that - Well, that's a deal breaker.

When I used the extends option for the first time (not long ago), I actually tried to pass the value true. After reading in your response that you disagree about the intuitiveness, I tried to think why I did it. This is what I came up with ...

Other tools using this behavior, make it expected. e.g. .jscsrc, .editorconfig, .gitignore, ...

Other JSHint options usually accept booleans. The ones that do accept strings, usually also accept booleans and in that case, using a string usually invokes a special or less common behavior. This makes the booleans expected, especially for invoking the common behavior.

The common usage is usually expected to have/get a shortcut, or even be/become the default. Since it is rare to see a project/team using cascading .jshintrc files take advantage of the freedom that passing paths as the extends option offers over booleans (i.e. skipping a .jshintrc file in an ancestor directory), the suggested behavior for booleans is actually the common usage. It even seems like most developers have no need for this freedom or just don't think like that, making me believe this behavior will be widely adopted.

The name extends doesn't necessarily suggest a file path is due as the value, so when writing your first .jshintrc file one will probably have to read the documentation. I'm mentioning this because of the explicitness/communication issue you mentioned - I agree that when someone who doesn't know the extends option will open a .jshintrc file and see a file path as the value he would know what is happening, but considering all the above, I think the understanding of what a boolean value means is not far behind, if at all (he might know one of the tools I mentioned before).

[jugglinmike]

So, I'll try to convince you a bit more (hope that's OK).

Of course it is! (Just as long as you don't mind occassional delays on my part)

Other tools using this behavior, make it expected. e.g. .jscsrc,
.editorconfig, .gitignore, ...

My objection applies to the behavior in JSCS and EditorConfig, as well: the
functionality allows projects to implicitly (and maybe even accidentally) rely
on configuration that exists outside of the project itself. I make an exception
for git because of a unique aspect of its implementation. From the page you
cited:

with patterns in the higher level files (up to the toplevel of the work tree)
being overridden by those in lower level files down to the directory
containing the file

So git configuration lookup automatically stops at project boundaries.
Unfortunately, this is not a behavior we can recreate for JSHint because we
have no equivalent to git's work tree.

This makes the booleans expected, especially for invoking the common
behavior.

I appreciate your experience as a user of JSHint, but that expectation only
comes from long-term use. Developers new to JSHint would have no reason to
share it.

To say that the value true means "invoke the common behavior" does not really
mean much. The pattern starts to have a little meaning when limited to the
context of JSHint's so-called "value options"; there, it usually means, "use
the most strict interpretation of this rule". That concept doesn't really map
to "extends".

I think the understanding of what a boolean value means is not far behind, if
at all (he might know one of the tools I mentioned before).

The ESLint project has a similar purpose as this one, and so it's probably wise to consider how the maintainers and users have handled this issue. The behavior you are requesting was initially the default in ESLint, but as they found, it is not without its share of edge cases, and that project now supports explicit configuration file extensions.