eslint: capitalized-comments conflicts with default-case ("// no default")

Let’s go ahead and implement the original proposal (to just make “// no default” case-insensitive), and then maybe we can reconsider making all regexes case-insensitive later.

(EDIT 2016-11-29T19:17:00Z: Proposal now favors allowing full case insensitivity for default pattern only. No changes will be proposed for custom commentPatterns.)

Here’s a proposal (and I’ll champion this).

What rule do you want to change?

default-case

Does this change cause the rule to produce more or fewer warnings?

Fewer.

How will the change be implemented? (New option, new default behavior, etc.)?

New default behavior: Make the “no default” default comment pattern case-insensitive in the default-case rule.

Please provide some example code that this change will affect:

// Example 1
switch (foo) {
    case 1:
        doSomething();
        break;
    // No default
}

// Example 2 (current tolerated comment)
switch (foo) {
    case 1:
        doSomething();
        break;
    // no default
}

// Example 3 (variant capitalization)
switch (foo) {
    case 1:
        doSomething();
        break;
    // NO Default
}

What does the rule currently do for this code?

Example 1: Warns on the // No default comment Example 2: Does not warn on the // no default comment Example 3: Warns on the // NO Default comment

What will the rule do after it’s changed?

Example 1: No longer warn on // No default Example 2: Continue to not warn on // no default Example 3: No longer warn on // NO Default comment

@platinumazure Sure thing -> just opened #7742.

This issue is already accepted, but what do people think about just making all commentPattern regexes case-insensitive? This would be a backwards-compatible change, and I can’t imagine that a project would care about the casing of a “no default” comment in general enough to only count the comment if it had a particular case.

Sorry for not being clearer! That’s correct, I’m 👍 for your proposal.

Alternatively, we could allow this:

{
  "commentPattern": "/foobar/i"
}

In other words, we could detect when the pattern is surrounded by slashes, and treat everything after the slashes as a regex flag.

@Alaneor That sounds reasonable. In order to make that work with the configurable comment pattern already in the rule, though, we might need to support a new schema option to specify whether the comment pattern is case-insensitive or not (since regex literals, including with flags, are not available in JSON/YAML).

If others on the team seem interested, I can expand my proposal to include that new schema option, and then it would simply be a matter of having the rule default to case-insensitivity unless a commentPattern is present in the user’s schema, in which case it would default to case-sensitivity (for backwards compatibility reasons) unless the user specifies they want case insensitivity.

Then again, we could make the default case-insensitive and wait for users to request a case insensitivity option for their custom patterns later…

I would propose making the whole check case-insensitive. Someone just might want to use // NO DEFAULT to make it really obvious.

If you use the options available in default-case, you can change the accepted pattern to support the capitalized case.

That said, I wouldn’t be against loosening the default pattern to allow the capitalized “No default” either.