[[FIX]] Skip undefined env in getHomeDir()

[joyeecheung]

Skip calling fs.existsSync() on an environment variable when it is undefined.

Discovered during the CITGM run of nodejs/node#18308

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1229/nodes=fedora-latest-x64/testReport/junit/(root)/citgm/bluebird_v3_5_1/

Before this patch, if fs.existsSync(undefined) throws, the test would fail like this:

▶ PATH=/Users/joyee/projects/node:$PATH ../node/deps/npm/bin/npm-cli.js run test

> jshint@2.9.5 pretest /Users/joyee/projects/jshint
> node ./bin/jshint src && jscs src

fs.js:251
    throw err;
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be one of type string, Buffer, or URL
    at Object.fs.existsSync (fs.js:434:3)
    at getHomeDir (/Users/joyee/projects/jshint/src/cli.js:112:12)
    at findConfig (/Users/joyee/projects/jshint/src/cli.js:83:14)
    at Object.getConfig (/Users/joyee/projects/jshint/src/cli.js:519:52)
    at /Users/joyee/projects/jshint/src/cli.js:643:43
    at Array.forEach (<anonymous>)
    at Object.run (/Users/joyee/projects/jshint/src/cli.js:642:11)
    at Object.interpret (/Users/joyee/projects/jshint/src/cli.js:756:18)
    at Object.<anonymous> (/Users/joyee/projects/jshint/bin/jshint:3:26)
    at Module._compile (module.js:661:30)

[joyeecheung]

Side note: I was a bit surprised that JSHint is not in the CITGM list, is there a previous discussion about this? (Have not found anything by searching in the CITGM repo) Would you consider joining that list? JSHint seem to meet a lot of requirements of CITGM.

[coveralls]

Coverage remained the same at 98.752% when pulling c99c1b1 on joyeecheung:fix-fs-exists-undefined into 157a508 on jshint:master.

[coveralls]

Coverage remained the same at 98.752% when pulling c99c1b1 on joyeecheung:fix-fs-exists-undefined into 157a508 on jshint:master.

[jugglinmike]

Thanks for the patch! And for the recommendation. Including JSHint in the citgm project sounds like a great idea! I'm not sure if JSHint's non-free license disqualifies it, though, so I've filed an issue to find out.

We'll want to fix this bug regardless, though. Would you mind adding a test so that we don't regress?

[joyeecheung]

@jugglinmike I believe this test

jshint/tests/cli.js

Lines 526 to 533 in 157a508

	testNoHomeDir: function (test) {
	var prevEnv = {};
	// Remove all home dirs from env.
	[ 'USERPROFILE', 'HOME', 'HOMEPATH' ].forEach(function (key) {
	prevEnv[key] = process.env[key];
	delete process.env[key];
	});

already severs as a regression test, since it makes all the home env variables undefined. Without this patch, if node throws on fs.existsSync(), this test would fail. With this patch the test passes. What do you think?