Add no-console-spaces rule

[MrHen]

A few questions:

1. I chose to use .trim() to apply the fix. This removes all whitespace -- including \t and \n. In theory we could expose a setting to chose what values to remove but then we'd probably want to upgrade to something like lodash.trim.

2. Similarly, .trim() also removes preceding whitespace. There could be a setting to switch to trimEnd instead?

3. This implementation explicitly ignores template literals. It wouldn't be a big deal to add them but the previous decision to remove \n makes some of the fixes awkward. Maybe we want to include template literals in the fix but ignore multiline literals?

4. I assumed this rule should be enabled in the recommended config.

5. Any other documentation examples and / or tests I should include?

6. Thoughts on the rule name? I personally prefer no-console-spaces or no-console-whitespace because I don't like prepositions in names when they can be avoided.

Fixes #175.

[sindresorhus]

1. It should only remove spaces. If there's are newlines, it's no longer likely a mistake and we should not interfere. The intention of the rule is to catch actual mistakes, not prevent valid usage of spacing and newlines.

2. I think it should only remove one space before and/or after the argument. Anything else is most likely not a mistake.

   Should fail:

   console.log('foo ', ' bar');
   console.log('foo', ` bar`);

   Should pass:

   console.log('foo  ', 'bar');
   console.log('foo', 'bar\n');
   console.log(' \nfoo', 'bar');
   console.log(`
   foo`, 'bar');

3. Only single-line template literals.

4. Yes

5. Looks pretty good, but easier to comment about that when the implementation is closer to being done.

6. no-console-spaces sounds good.

[MrHen]

The other question is if we should worry about someone shadowing console. The eslint no-console rule has an example of how to survive shadowing. (Maybe work on that as a follow up?)

In any case, I'll start plugging away at your requests. 😄

[sindresorhus]

The other question is if we should worry about someone shadowing console. The eslint no-console rule has an example of how to survive shadowing. (Maybe work on that as a follow up?)

Nah. That's an edge-case. Would be nice to solve eventually, but can be a follow-up.

[sindresorhus]

Can you rebase from master? I just added back linting (It was previously removed because of problems with ESLint 4). And then run npx xo --fix and fix any lint issues.

[sindresorhus]

There are many more log methods: https://developers.google.com/web/tools/chrome-devtools/console/console-reference We only need to support the ones that accept multiple arguments.

[sindresorhus]

You might (or may not) find these useful:

• https://mysticatea.github.io/eslint-utils/guide/getting-started.html
• https://github.com/jfmengels/eslint-ast-utils

[sindresorhus]

There are many more log methods: https://developers.google.com/web/tools/chrome-devtools/console/console-reference We only need to support the ones that accept multiple arguments.

[MrHen]

I added debug and info. The only other multi-parameter is group which isn't really the same. I can add that as well if you want.

[MrHen]

@sindresorhus Updated PR.

• Will now only fail / fix exactly one space on either side of the string
  • Of note, I decided to leave console.log(' '); alone mostly because I was lazy and didn't want to add an explicit check for it. If you have strong feelings I can add a check -- but in a certain sense I'm not sure we really want to replace console.log(' '); with console.log('');.
• Added support for template literals
• Rebased and fixed lint errors
• Added missing documentation
• Added missing console methods (see comment)

Let me know if you want any other changes.

[MrHen]

@sindresorhus gentle bump in case the earlier notification got lost in the wind.

[sindresorhus]

This is too generic. Should be something more like:

Do not include leading/trailing space in console.log parameters.

[sindresorhus]

typo, trailing

[sindresorhus]

Seems like using .startsWith()/.endsWith() and then .slice(1) or .slice(-1) would be simpler and more readable.

[sindresorhus]

Sorry about the delay. This got lost in a tab somewhere.

[sindresorhus]

I don't think these should be failures:

• console.log("abc ");
• console.log(" abc");
• console.log(" abc ");

They are not really mistake. The mistake is not realizing multiple arguments are joined with a space.

Meaning, these should not be failures either:

• console.log(" abc", "abc"));
• console.log("abc", "abc "));
• console.log(" abc", "abc "));

[MrHen]

@sindresorhus No worries. I made the changes requested but travis is failing due to the following:

  Error thrown in test:
  TypeError {
    message: 'process.stdout.getColorDepth is not a function',
  }

I'm not entirely sure how that is relevant to my changes but I can dig into it later today. Figured it out; was being an idiot. The error message in travis wasn't very helpful though.

[sindresorhus]

This is looking great now. Thanks for your patience. I think we ended up with something really good :)