[[FIX]] Removed warning message W041

[sunilkumarc]

This commit removes the warning message W041 altogether from the jshint
project.

Fixes #3109

[coveralls]

Coverage decreased (-0.005%) to 97.749% when pulling 1e8ab37 on sunilkumarc:fix-3109 into 6c34960 on jshint:master.

[jugglinmike]

Thanks for the patch, @sunilkumarc! I've left some feedback for you as in-line comments. Beyond those, could you remove the changes to the files in the dist/ directory? Those files are re-written during the release process (not for each change to the source).

[jugglinmike]

Could you declare this property as null instead? Doing so is consistent with how we've handled invalid warnings in the past, and it makes it more clear to future readers that the "missing" warning is intentional.

[jugglinmike]

This change effectively disables the unit test suite. Since we want to continue
running tests for all the rest of the functionality, we'll have to be more
specific about which tests are invalidated by this change. Could you
re-introduce this code and instead remove the specific assertions that relate
to warning W041?

[sunilkumarc]

Hi @jugglinmike . I have made all the changes you suggested. I have one question regarding the last suggestion, that is, disabling unit test suites to avoid only W041 assertions.

My question is, there are some unit tests with only assertions for W041 messages. In such a case should I remove the entire unit test or just the assertions?

For example, in the below code, should I just remove just all the addError assertions or the entire unit test itself?

TestRun(test)
    .addError(1, "Use '===' to compare with 'null'.")
    .addError(2, "Use '===' to compare with 'null'.")
    .addError(3, "Use '!==' to compare with 'null'.")
    .addError(4, "Use '!==' to compare with 'null'.")
    .test(code, {es3: true});

[sunilkumarc]

Hi @jugglinmike. Did you get time to look at my above comment?

[jugglinmike]

No, not yet.

[sunilkumarc]

No problem. You can check and reply in your free time :)

[jugglinmike]

My question is, there are some unit tests with only assertions for W041
messages. In such a case should I remove the entire unit test or just the
assertions?

Good question. It's probably safest to retain the test itself. It may be
confusing to future contributors, though, so lets add a comment explaining why
it is still there. Maybe something like, "This test previously asserted the
issuance of warning W041. W041 has since been removed, but the test is
maintained in order to discourage regressions."

[coveralls]

Coverage decreased (-0.08%) to 97.676% when pulling 6135ad5 on sunilkumarc:fix-3109 into 6c34960 on jshint:master.

[sunilkumarc]

Hi @jugglinmike. I have made the changes you suggested and pushed the changes. Please check now.

[jugglinmike]

This is looking good, @sunilkumarc! In reviewing these changes, I've learned
that the "relaxing" option named eqnull is tied up in all of this. That
increases the scope a bit, so I'd like to rope @rwaldron in to the conversation
to get his input.

Rick: for context, here's what we know:

• JSHint provides an "enforcing" option named eqeqeq. When the user
  explicitly enables it, JSHint will warn about usage of the != and ==
  operators. Otherwise, JSHint generally tolerates those operators.
• However, even when eqeqeq is not enabled, JSHint currently issues warning
  W041 if either side of the relation is a falsey literal values.

This situation is confusing for users because even if they "disable" eqeqeq
(which, as an enforcing option, is disabled by default), JSHint will continue
to instruct them to use strict comparison operators in this specific case.

What I've just learned, though, is that a second linting option, eqnull,
actually fine-tunes JSHint's behavior within that confusing special case. By
enabling it, users can opt out of the confusing warning whenever the offending
"falsy" value is the null literal.

This has expanded the scope of the "fix" I suggested--we are now considering
making an existing linting option irrelevant. To be clear: that is not a
threat for compatibility. Like any other "enforcing" option, we can disable it
in a patch release and know that no new warnings will be produced. But it does
make this change seem more substantial, so I wanted to call that out.

Maybe I'm just being sentimental, though. If a linting option is only relevant
in a situation we previously determined to be confusing, then I suppose we
should welcome the opportunity to remove it. Doing so makes using JSHint just a
little simpler, even if it does relax the requirements a bit.

So what do you think, @rwaldron and @sunilkumarc? Should we proceed with this
change as it is? (If so, I'd recommend updating the documentation of eqnull
to explain that the option has no effect and is maintained for compatibility
purposes.)

[jugglinmike]

Sorry for the delay here, @sunilkumarc. I happen to know that @rwaldron has had
his hands full these past few days. I'll reach out to him offline and see what
he thinks.

[sunilkumarc]

No problems at all @jugglinmike :) I can wait.

[rwaldron]

Thanks @sunilkumarc and @jugglinmike

[sunilkumarc]

Thanks @rwaldron. Thanks a lot @jugglinmike, for all the help. 👍

[jugglinmike]

No problem at all, @sunilkumarc! Thank you for your contribution and your patience!