golang-ci: Enable lll (long lines) linter and fix issues

[3v1n0]

Set the long lines linter to block lines longer than 120 chars an fix the cases in which we were not respecting this limit.

This was somewhat mentioned during the sprint, and I wanted to finally tackle it :)

[denisonbarbosa]

I like the addition, but there's some shady stuff in the diff. Can you take a double look at the corrections and make sure we don't have some weird formatting?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 82.30769% with 23 lines in your changes missing coverage. Please review.

Project coverage is 82.91%. Comparing base (36f3d0a) to head (852d3ec).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/users/manager.go	33.33%	6 Missing ⚠️
internal/brokers/localbroker.go	0.00%	4 Missing ⚠️
pam/internal/adapter/authentication.go	42.85%	4 Missing ⚠️
pam/internal/adapter/authmodeselection.go	0.00%	3 Missing ⚠️
cmd/authd/daemon/config.go	50.00%	2 Missing ⚠️
examplebroker/broker.go	85.71%	2 Missing ⚠️
pam/internal/adapter/commands.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
- Coverage   82.99%   82.91%   -0.08%     
==========================================
  Files          80       80              
  Lines        8584     8633      +49     
  Branches       75       75              
==========================================
+ Hits         7124     7158      +34     
- Misses       1129     1141      +12     
- Partials      331      334       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adombeck]

I would prefer to not enforce the line length via the linter. I'm in favor of agreeing on a guideline regarding line length (e.g. "try to keep lines shorter than 120 chars, in multi-line comments shorter than 80") but there are common cases where I think a long line actually makes the code more readable than a introducing a line break. For example when printing log messages, I prefer:

log.Warningf(context.Background(), "Failed to get current executable path, not adding it as a config dir: %v", err)

over

log.Warningf(context.Background(),
    "Failed to get current executable path, not adding it as a config dir: %v", err)

And I very much prefer not wrapping the log message itself, because that makes it harder to find the code which prints a given message.

Another case where I prefer longer lines is function signatures. For example, this PR contains this diff:

-func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) {
+func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (
+       sessionID, encryptionKey string, err error) {
        sessionID = uuid.New().String()
        info := sessionInfo{

With that change, the second line of the function signature has the same indent as the function body, which makes it hard to recognize it as part of the function signature.

[3v1n0]

I've not played too much with linter settings but I feel it could be possible to avoid specific cases.

I can understand the functions signatures though, but I still feel that logging is better to read when split. In case you rely on grep using -N isn't a big deal I think.

[adombeck]

I can understand the functions signatures though, but I still feel that logging is better to read when split. In case you rely on grep using -N isn't a big deal I think.

My grep doesn't know about a -N option:

$ grep -N
grep: invalid option -- 'N'

Also I read and navigate code in an IDE, which does not find strings when they are separated by a newline (and other characters to concatenate the two substrings).

[3v1n0]

My grep doesn't know about a -N option:

I meant -<N>, so for example -2 to get the 2 lines around the grepped code.

[adombeck]

I meant -, so for example -2 to get the 2 lines around the grepped code.

That doesn't help me when I have an error message Permission denied: You have insufficient privileges to access this resource and the code which prints it is:

log.Error("Permission denied: You have insufficient privileges to " + \
"access this resource")

When I search for "You have insufficient privileges to access this resource", I won't find the code which prints it.

[didrocks]

When I search for "You have insufficient privileges to access this resource", I won't find the code which prints it.

Fully agreed this is a desired behaviour and my issue too with splitting such lines.