gavv / returnstyles Goto Github PK

Implement two new options allowing to specify regexp to exclude matching files and functions from diagnostics.

Options should be added to:

• command-line
• YAML config
• README

To implement -exclude-funcs, it should be enough to match function name in skip() function.

For -exclude-files, a more complicated logic is probably required. We don't have file information in skip(), so we'll need to obtain it somehow, or probably adjust AST traversal algorithm a bit.

We should also add two new tests for each option, which check that diagnostic is NOT reported for ignored files / functions.

It would be awesome to add support for golangci-lint

Add new boolean option -detect-config.

If the flag is specified, automatically search for file named .returnstyles.yml starting from current directory and recursively in parent directories.

If the file is found, load config from it. If the file is not found, report an error and exit.

If user specifies both -config and -detect-config, report error and exit.

We also need to add a test for this new option and document it in README.

I'd like to propose --no-else-after-return option

// bad
func bar() (a int, b int) {
    if cond() {
        a = 11
        b = 22
        return a, b
    } else {  // lint: disallow else after return
        return 11, 22
    }
}

convert to this:

// good
func bar() (a int, b int) {
    if cond() {
        a = 11
        b = 22
        return a, b
    }

    return 11, 22
}

Teach linter to recognize two comment directives:

• Diagnostics for foo should be disabled (both for foo signature and for any return statements in its body).

  //returnstyles:ignore
  func foo() {
      ...
  }

• Diagnostics for bar and its return statements should be enabled, even if otherwise we would disable it.

  //returnstyles:enforce
  func bar() {
      ...
  }

  See:

  • #2
  • #3

exhaustive linter has similar diagnostics directives, so we can look for hints on implementation in its source code.

We should add tests for both directives and document them in README.

Currently we have Linux and macOS. We need to add Windows too and fix any issues arising.

Currently we unconditionally skip cgo functions:

func skip(funcNode ast.Node) bool {
	switch funcNode := funcNode.(type) {
	case *ast.FuncDecl:
		if strings.HasPrefix(funcNode.Name.Name, "_Cfunc_") {
			return true
		}
	}

We should also add new option -include-cgo to disable this behavior:

• to command-line options
• to YAML config
• to README

If this option is set, we should NOT skip cgo functions.

We should also add two new tests:

• with cgo, without -include-cgo option; diagnostic should NOT be printed for cgo functions
• with cgo, with -include-generated option; diagnostic should BE printed for cgo functions

Adjust linter to detect generated files and ignore them by default.

The algorithm can be adopted from exhaustive linter: https://github.com/nishanths/exhaustive/blob/master/comment.go. We can replicate its isGeneratedFile() function and use it in run() to skip generated files.

Steps:

• in run(), pass stack to skip() function
• in skip(), get the very first stack frame and cast it to ast.File
• do a check similar to isGeneratedFile from exhaustive to determine if file is generated
• skip file if it is generated

We should also add new option -include-generated to disable this new behavior:

• to command-line options
• to YAML config
• to README

If this option is set, we should NOT skip generated files.

We should also add two new tests:

• with generated code, without -include-generated option; diagnostic should NOT be printed for generated code
• with generated code, with -include-generated option; diagnostic should BE printed for generated code