Antonboom / testifylint

The Golang linter that checks usage of github.com/stretchr/testify.
https://github.com/stretchr/testify
MIT License
101 stars 8 forks source link

require-error-is-as: possible broken import when fixing #127

Closed ccoVeille closed 5 months ago

ccoVeille commented 5 months ago

-fix may imply that errors package is no longer needed

Here is an example

package main

import (
    "errors"
    "os"
    "testing"

    "github.com/stretchr/testify/assert"
)

func TestWhatever(t *testing.T) {
    _, err := os.UserHomeDir()
    assert.True(t, errors.Is(err, os.ErrDeadlineExceeded))
}

when using -fix, testifylint will replace errors.Is by assert.ErrorIs. Here is the result.

package main

import (
    "errors"
    "os"
    "testing"

    "github.com/stretchr/testify/assert"
)

func TestWhatever(t *testing.T) {
    _, err := os.UserHomeDir()
    assert.ErrorIs(t, err, os.ErrDeadlineExceeded)
}

But this file is invalid: the errors package is no longer needed, launching a goimport on it fix it

ccoVeille commented 5 months ago

Please note the bug may also occurs with formatter checker, if fmt package is no longer necessary

Antonboom commented 5 months ago

I admit that this is possible.

  1. testifylint doesn't have to deal with formatting the entire file
  2. I don't want to "burden" the linter with logic to detect the use of imports
  3. The standard pattern of linters usage in combination of go fmt, gofumpt, gci, goimports, etc.
  4. IMHO, it's better to have not completed fix that no fix at all. Especially when the rest issues are detected by compiler

So, nothing to do here.

ccoVeille commented 5 months ago

OK for me. As long, it's clear and documented.