walle / lll

Line length linter
MIT License
65 stars 9 forks source link

lll should not complain about long strings #11

Open Zamiell opened 4 years ago

Zamiell commented 4 years ago

One of, if not the most popular style guide in the world is the Airbnb JavaScript Style Guide, which represents best-practice across a lot of the JavaScript and TypeScript ecosystem.

In the style guide, it mentions:

6.2 Strings that cause the line to go over 100 characters should not be written across multiple lines using string concatenation.

Why? Broken strings are painful to work with and make code less searchable.

// bad
const errorMessage = 'This is a super long error that was thrown because \
of Batman. When you stop to think about how Batman had anything to do \
with this, you would get nowhere \
fast.';

// bad
const errorMessage = 'This is a super long error that was thrown because ' +
  'of Batman. When you stop to think about how Batman had anything to do ' +
  'with this, you would get nowhere fast.';

// good
const errorMessage = 'This is a super long error that was thrown because of Batman. When you stop to think about how Batman had anything to do with this, you would get nowhere fast.';

This policy seems sensible. With the "bad" code example above, grepping through a project for "thrown because of Batman" would return 0 results.

One other problem with using string concatenation is that realignment is tedious when you have to remove a sentence. And it introduces unnecessary Git noise. e.g.

msg := ""Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor " +
    "incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud " +
    "exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure " +
    "dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."

-->

// We remove the second sentence, but now the entire code block has to be readjusted
msg := ""Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor " +
    "incididunt ut labore et dolore magna aliqua. Duis aute irure dolor in reprehenderit in " +
    "voluptate velit esse cillum dolore eu fugiat nulla pariatur."

In JavaScript/TypeScript land, the most popular linter is eslint, which is similar to lll in that it has a max-len rule. It has an ignoreStrings flag which is set to true by default - a sensible default.

The implementation for this is located here: https://github.com/eslint/eslint/blob/master/lib/rules/max-len.js#L314-L315 As you can see, it is extremely simple.

In Golang land, I think the practice of ignoring strings should also apply, for precisely the same reasons. I was curious to see if other big Go projects were breaking up long strings, so I took a look at Kubernetes, which is probably the biggest. I was only able to find one instance of concatenated strings. Instead, most code is written like like this.

Similar to eslint, is it possible to add an ignoreStrings flag to lll? Currently, I have gotten so fed up with adding //nolint: lll everywhere that I have stopped using the linter, but it is a great tool and I would love to be able to use it again in the future.

orsinium commented 3 years ago

One more reason:

Statements longer than 80 columns will be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. <...> However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them.

agnjunio commented 3 years ago

I also believe that Struct tags should be ignored too (they are also strings)

mitar commented 1 year ago

I made https://github.com/golangci/golangci-lint/issues/3983 to track this in golangci-lint repository.