New Feature: support nolint range

[sanposhiho]

Hi team :D

Fixes #129

Support the new feature nolint range.
Now, we can ignore reports with nolint-begin and nolint-end

func Save() {
  //nolint-begin:lll
  very long line of code 
  another very long line of code
  one more 
  ...
  //nolint-end:lll
}

I've opened it early to hear any feedback on the syntax. I'll open it as a formal PR once I've written some tests and can get everyone to agree on the above syntax.

[ldez]

Hello,

I'm not sure about the syntax with a hyphen.

Maybe nolint:begin:lll or nolint:lll:begin.
But there is also interaction with the nolint syntax for multiple linter.

I'm also not sure that discuss that in a PR is a good idea.

🤔

[sanposhiho]

Hi

Maybe nolint:begin:lll or nolint:lll:begin.

Yes, I also considered the syntax you mentioned.
But, since not all users know the names of all linter in golangci-lint, I thought that the syntax nolint:begin: or nolint:begin(the case user want to ignore all linter's reports) might cause users to think that they are ignoring the linter named "begin". 🤔 💭

I'm also not sure that discuss that in a PR is a good idea.

Yeah, If there are some problems with this syntax or other ideas about it, discuss it in the issue 👍

[bombsimon]

Is this really needed, this can be solved with an anonymous block.

package main

import "fmt"

func main() {
	//nolint:forbidigo // Let's OK them here
	{
		fmt.Println("x")
		fmt.Println("x")
		fmt.Println("x")
	}

	// Not here.
	fmt.Println("x")
}

$ golangci-lint run --no-config --disable-all --enable forbidigo
main.go:14:2: use of `fmt.Println` forbidden by pattern `^fmt\.Print(|f|ln)$` (forbidigo)
        fmt.Println("x")
        ^

[ldez]

Sorry, I forget to answer.

My main concern with that related to #1658
I think this issue is more important than nolint range, and it can be difficult "to migrate" this nolint annotation in the future.

[sanposhiho]

Is this really needed, this can be solved with an anonymous block.

As you know, {} means scope. So I think nolint with {} can only solve certain cases.
For example, If a user wants to nolint lines only 2-5 of example1, nolinting with {} is not possible as in example2.

example1:

func main() {
	var superLongName int  // lll reports "this is long line"
	var superLongName2 int // lll reports "this is long line"
	var superLongName3 int // lll reports "this is long line"
	var superLongName4 int // lll reports "this is long line"

	println(superLongName) // lll reports "this is long line"
	fmt.Println("Hello, playground")
}

example2:

func main() {
        //nolint: lll
	{
		var superLongName int
		var superLongName2 int
		var superLongName3 int
		var superLongName4 int
	}

	println(superLongName) // ERROR undefined: superLongName
	fmt.Println("Hello, playground")
}

As the other case, users can't do nolint across multiple functions or scope.

[sanposhiho]

My main concern with that related to #1658
I think this issue is more important than nolint range, and it can be difficult "to migrate" this nolint annotation in the future.

Hmm, I didn't know about that issue, thanks.
Yeah, as you say, I think that issue has a higher priority than this nolint range feature.

I think there are several actions that can be taken here, and I'd like to hear your opinions..

• Give up on this feature? (i.e. don't implement it in the future, i.e. close Idea: support "nolint ranges" #129 )
• Discuss it again after adhere to the widespread comment directive format #1658 is completely resolved?
• Discuss here(or in the issue Idea: support "nolint ranges" #129) a syntax for nolint range that does not deviate from go standard?
• other actions...?

[ldez]

My opinion:

• Discuss it again after adhere to the widespread comment directive format #1658 is completely resolved
• Discuss in issue Idea: support "nolint ranges" #129 a syntax for nolint range that does not deviate from Go standard

[sanposhiho]

I agree with you.
So, if no one else has some opinion, let's wait #1658 to be resolved

[ldez]

I will close the PR and keep issue #129 to track the topic.