Linting

[Marcello H]

Since we have great linters, I wonder why it seems that the Go source itself isn't linted?
(Because when I do lint it, I see a lot of suggested improvement)

[Ian Lance Taylor]

On Fri, Dec 9, 2022 at 5:38 AM Marcello H <marc...@gmail.com> wrote:
>
> Since we have great linters, I wonder why it seems that the Go source itself isn't linted?
> (Because when I do lint it, I see a lot of suggested improvement)

1) Linters are optional and opinionated, and it kind of matters which
linter you mean.

2) Changing working code to make it cleaner should only be done if the
change makes the code significantly better in some way (faster, easier
to read, etc.). It shouldn't be done just to satisfy a linter. The
most likely effect of changing working code is to break such that it
no longer works.

Ian

[Brian Candler]

On Friday, 9 December 2022 at 19:09:34 UTC Ian Lance Taylor wrote:

1) Linters are optional and opinionated, and it kind of matters which
linter you mean.

And of course, go itself already comes with the grandaddy of all linters - "go vet".

[Marcello H]

I agree that not all linting is useful, but when I see certain outcome that is questionable, I would be investigating that a bit more if it was my code.
For example:
There is code that uses deprecated functionality, that could easily be changed to the newer version of that.

There are:
- lots of ineffective assigns to be found (which means for some occasions extra work for nothing)
- structs that could be made more memory efficient
- type assertions that should be checked

- unnecessary conversions
- parts with unchecked errors

- parts that have a high cognitive code complexity

  > for example "go/src/cmd/compile/internal/ssa/regalloc.go"

  func (s *regAllocState) regalloc(f *Func)
  who can read this?

So yes, linters can be useful as well

Op zaterdag 10 december 2022 om 10:30:00 UTC+1 schreef Brian Candler:

[Brian Candler]

The question remains, which linter(s) are you using?

[Marcello H]

golangci_lint and my own (for the cognitive code complexity)

Op za 10 dec. 2022 om 13:17 schreef Brian Candler <b.ca...@pobox.com>:

The question remains, which linter(s) are you using?

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/MM3W5oNw4-0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/bd99406a-130d-4405-bbc6-4ff6ef37eca9n%40googlegroups.com.

[Dan Kortschak]

On Sat, 2022-12-10 at 14:44 +0100, Marcello H wrote:
> golangci_lint ...

>
> Op za 10 dec. 2022 om 13:17 schreef Brian Candler
> <b.ca...@pobox.com>:
> > The question remains, which linter(s) are you using?

golangci_lint is not really an answer to this question since it's just
a collection of linters, many of which have arguable utility/value.

[Marcello H]

Ok, but does my answer really matter? Because in the end it's what any linter will tell us about the software.
And since it is not hard to do, my suggestion always is to make use of some of the great linters that exist.

In a new project I always start off with all the linters (in golangci_lint) active, and then deactivate the ones that I don't want.
You can also do it the other way around, just a matter of "how to begin with linting".
The advantage of using some linter would be that it is also a kind of quality fixation on the code.

Perhaps deciding on which linter(s) to use at first and then slowly "inject" more.

This post is not about finger-pointing btw, because if needed, I would offer my help with this.
But I won't touch the functions with the high cognitive complexity ;-)

Op za 10 dec. 2022 om 22:34 schreef 'Dan Kortschak' via golang-nuts <golan...@googlegroups.com>:

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/MM3W5oNw4-0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9827b253f9d8cc8c821874675adad2130612060e.camel%40kortschak.io.

[Amnon]

If you want, run any linters of your chosing on the Go standard library,
go through the reports, and see if any are valid - which expose real problems,
then feel free to report them here, or submit bug reports. 

[Ian Lance Taylor]

On Sun, Dec 11, 2022, 2:03 AM Marcello H <marc...@gmail.com> wrote:

Ok, but does my answer really matter? Because in the end it's what any linter will tell us about the software.
And since it is not hard to do, my suggestion always is to make use of some of the great linters that exist.

In a new project I always start off with all the linters (in golangci_lint) active, and then deactivate the ones that I don't want.
You can also do it the other way around, just a matter of "how to begin with linting".
The advantage of using some linter would be that it is also a kind of quality fixation on the code.

Perhaps deciding on which linter(s) to use at first and then slowly "inject" more.

This post is not about finger-pointing btw, because if needed, I would offer my help with this.
But I won't touch the functions with the high cognitive complexity ;-)

We do use a linter on the Go standard library: go vet.  When some change in go vet causes it to report errors in existing code, we fix them.

Ian

[Marcello H]

Ok, I made a list with functions that are hard to understand because of the complexity. This is a calculated cognitive code complexity, but it serves me quite well when doing code reviews.
The author of such a function might think that it's pretty easy to understand, but code complexity is how others look at code.
(code ideally is maintainable by others as well)

Following the "Go Style Guide", it should have Clarity, Simplicity, etc.

I've put the threshold at 25 (which I normally have at 12). So everything above a complexity of 25 is reported.

I also think that functions with a higher number of lines are also harder to understand, maintain etc.
I've put the threshold at 150 (which I normally have at 50). So everything above 150 lines is reported.

Both lists are attached, but what should I do with them?

Op zo 11 dec. 2022 om 21:42 schreef Ian Lance Taylor <ia...@golang.org>:

[Brian Candler]

I'd say that in an entire programming language and runtime environment like Go, there is much which is inherently complex.  Just think about things like garbage collection and asynchronous preemption.  A linter which creates dumb metrics like "number of lines of code in a function" is unable to appreciate the semantics of what is required to perform the job.

Also consider that what you are saying *could* be construed as an insult to the incredibly clever and dedicated people who have worked on this for years. The implication is "you're not doing a very good job", or "why didn't you think of using a tool like this?" (duh!)

With that in mind, I'd respectfully suggest that your starting point should be to pick one of these "over complex" functions, understand it fully, and rewrite it in what you consider to be a simpler/clearer way which does not break any functionality, *and* which does perform any worse than the original code (*).  Then you can submit it as a pull request for review.  Rinse and repeat.

(*) Even marginal performance regressions are going to hurt somebody somewhere.  There's much history in github issues that is worth reading - just search for "bench" or "benchmark".

[Marcello H]

I totally understand that you put the ball back to me. My message is not to blame anybody and I think I didn't do that. People might be offended, but I'm just the messenger.
The trick is, if we want software to survive, we should always check if it is understandable and maintainable and that is my message.
The people who were willing to write solutions for complex problems are really awesome, because they also made Go the way it is today.
But what happens if such a person is not able to do that anymore? Would others still comprehend what the original author had meant to do?
We can be ahead of that, by looking at useful insights, like code complexity or number of lines that will give us hints about what could be improved.

I've said before, I want to help, but won't touch overly complex code. Because for me it's a waste of time understanding the code,
while the original author can probably fix it in a breeze.

(This is why linting regularly is so important: it will warn the author something is off.)

I really hope nobody is offended and keeps up the good work.

Op di 13 dec. 2022 om 11:31 schreef Brian Candler <b.ca...@pobox.com>:

--

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/MM3W5oNw4-0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9d9147b0-3c4c-4be2-ac49-43e0dd13aac0n%40googlegroups.com.

[Ian Lance Taylor]

On Tue, Dec 13, 2022 at 2:31 AM Brian Candler <b.ca...@pobox.com> wrote:
>
> With that in mind, I'd respectfully suggest that your starting point should be to pick one of these "over complex" functions, understand it fully, and rewrite it in what you consider to be a simpler/clearer way which does not break any functionality, *and* which does perform any worse than the original code (*). Then you can submit it as a pull request for review. Rinse and repeat.

With respect, I'm going to suggest not doing that. If code is working
correctly, then changing it, even to simplify it, may cause it to stop
working correctly. In fact, this is more likely to happen if the
original code is complex. The best approach to complex working code
is to only change it if there is a good reason to do so: because the
requirements have changed, or because there is some way to make it run
measurably faster, or because it is a regular source of bugs.

Also I hope we can all agree that blind adherence to any specific
definition of complexity is misguided. For example, a function that
is basically a big switch statement with a bunch of cases is often
easier to understand as a single function rather than a collection of
different ones. I hope we can all agree that reducing the number of
code complexity reports to zero is not a goal. I'll note that the
list of complaints here seems to me to be biased toward breaking up
large functions into smaller functions. In some cases that can make
the code easier to understand. In other cases it does not. Judgement
is required.

Ian

[Marcello H]

I have no complaints, let me be clear about that. Just advice, for all I've learned in my cariere is that maintenance is happening more than the first version of something creative.
But I've still got hope that one day I might understand some complex coding that I didn't write myself.

I'm happy to help, if an author of something "complex" can guide me. The only thing I can do now, is be a messenger.

Op di 13 dec. 2022 om 20:52 schreef Ian Lance Taylor <ia...@golang.org>:

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/MM3W5oNw4-0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAOyqgcXGWbAEVy89cMFXmwS-%3DPYZ%3D6NEYQ-i6NnzyfMmZ8W7MA%40mail.gmail.com.

[Amnon]

This long discussion started with the question why the Go source is not linted. 

But I think the OP has answered his own question by providing us with a list of 963 places 
where the linter has decided that the code is deficient. 
But what is notable, is that he has not given a single example where any of these 963 warning has any value.
He has not taken even one of these warnings and show how "fixing" it leads to better code.
411 of these warnings are for functions which are longer than the 150 lines which are "allowed".
So I would ask: "allowed by whom?"
I am a big fan of short functions, as are the authors of the Go standard library. But surely the optimal length of a function depends
on the context. I am not too bothered, about the length of functions in the output of code generation, for instance. And I am OK with 
code with a very regular structure - like a switch statement handling many cases to take many lines.
Some of these linters do seem to be quite bureaucratic - imposing arbitrary rules irrespective of whether they make sense in the actual context.
Code written in corporations (because it usually is in corporations) where these linters are imposed, have a particular smell, with all kinds
of convolutions and ugliness designed to appease the linter.
If you see code where every line begins with 

    _, _ =

Or where you always see 

    defer func() { _ = res.Body.Close())() 

rather that 

   defer res.Body.Close()

you know that this is the product of a programmer toiling under the dictates of a linter.
Because if you are discussing some code with a knowledgeable reviewer, you can defend your decisions.
You can explain why writing to a byte.Buffer can not fail (unless we are out of memory, in which case any error 
handling will fail too). But if you are told off by a linter, there is no appeal. The linter has no discretion.
It is not open to discussion. It will not consider your arguments. Your job is to do what the linter tells you. 
Because the linter is always right. Because the computer is always right. And you, as a mere human, are always wrong.

I would say that go vet is an exception - its warnings almost always point to real problems in the correctness of code,
or unnecessary complexity. I often also run staticcheck on my own code. It often makes useful observations, some of which 
lead to useful changes. But beyond these two, linters often do more harm than good. One very high quality linter with 
few false positives is far more valuable that a large number of linters which make nonsensical complaints.
golangci_lint is particularly bad in this respect - making it easy to run loads of low quality linters, which cry wolf all the time and 

train programmers to ignore warnings.

So I would appeal Ian and the rest of the Go team, to resist the pressure to adopt these "great linters" - not to invest months of work
"fixing" these 963 "problems" identified by these linters. And focus on carrying on doing what you are doing - producing,
rocks solid releases, well thought out enhancements, and code which can serve as a template we can all emulate
to produce good, clear, idiomatic code. 

[Marcello H]

Sorry, but I didn't state (and so does the linter) that any code is deficient.

Normal usage of a linter is not to satisfy a linter, but to do some basic code review.

(Most new linters can skip certain code lines these days too.)

I know that  the number of lines in a single function is just a matter of taste.
But I also know that a function with too many lines is also harder to maintain.
If you study those functions that are "too long", you will find that it could probably be split up in a more readable and maintainable way.

The only worry I have is that complex code is very hard to maintain, perhaps only by a single person.

What happens if that person isn't available anymore? The best time to maintain code to  a more maintainable version, is at the moment of creation or not much later,

because only then it lives still "in the mind" of the creator. A linter can help detect those "blindspots" of a developer and can point out where those are.

With the new guidelines, one can expect to see the Go code as an example of those guidelines.
(one of them being maintainable software)

Does anybody have to fix all of it now? Of course not, but perhaps when a developer is touching one of those functions and understands what it does, then it is still the ideal time to rewrite it a bit.

But perhaps I'm just a sucker for quality and maintainability and I'm barking up the wrong tree.
I very much want Go to survive, so for me that means that people should not be afraid to maintain parts of the code.

(and that's why I said not to touch all those high complexity parts, because I find it highly confusing; so i'm not fit for that job.)

Op wo 14 dec. 2022 om 23:33 schreef Amnon <amn...@gmail.com>:

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/59fb5c6e-346c-4a3a-b1b4-b59b3b3510a2n%40googlegroups.com.

[Amnon]

> But perhaps I'm just a sucker for quality and maintainability and I'm barking up the wrong tree.

I don't think that you will find many people here who don't think that code quality and maintainability 
are important.

But I would disagree with the implicit assumptions that
code that "passes" a linter is of high quality

and that code that "fails" the linter is of poor quality

From my limited readings of the Go source code, I would say that the Go team's judgement of code quality is far beyond that
of any linter I have seen.