incu6us / goimports-reviser Goto Github PK

The only time I can get it to work is if the workdir is the go.mod root. If I am anywhere inside of the go project, it doesn't crawl up until it finds the go.mod, it just errors saying it wasn't able to open the go.mod.

The logic should probably be updated to be something like this:

1. If filepath is set, get the folder of that file and then crawl upwards
2. If filepath is not set, get the current workdir and then crawl upwards

Hello,

First and foremost thank you so much for building this awesome tool; I use it religiously.

Issue
I am using wasm with go and noticed that the syscall/js import isn't in the group i expected .. is this a bug?

• I expected it to be in the first group

e.g

import (
	"fmt"
	"time"

	"github.com/lestrrat-go/jwx/jwt"
	"syscall/js"

	"github.com/darwayne/project/blah/blah"
	"github.com/darwayne/project/internal/blah"
)

wrong to group "embed" lib with go 1.16

I would like to use this tool to format the imports for every file in my project. To do this, I have to write a script something similar to

for file in $(find . -name '*.go'); do
    goimports-reviser  -file-path $file -project-name $PROJECT_NAME
done

goimports supports this, for example simply running goimports -w . will recursively format imports for every go file in .

I would suggest a similar approach, allow the file-path argument to take a directory and format all files when given as an input, but will defer to code owners for what's best.

Thanks!

github.com/pkg/errors is archived and not maintained anymore since Go 1.13. See StackOverflow answer for details.

The project still uses this deprecated package in main.go, gen.go, and dir.go files by calling errors.WithStack. Returned errors from these files are simple enough to track and understand. It's not needed to print an extended stack trace. So, we can rewrite to use native errors and fmt.Errorf and avoid using github.com/pkg/errors.

What do you think?

in go 1.21.0, $ go mod init foo will create a go.mod file with go 1.21.0 in it, instead of go 1.21. in this case, goimports-reviser will show an error message:

go.mod:3: invalid go version '1.21.0': must match format 1.23

Hi Team,

I just want run this on the CI and check whether the goimports is run or not. If its not run I want to fail.

Is it possible to achieve this goimports-reviser

thanks,
Pradeep

With go1.21 the module version has to be in format X.Y.Z. See golang/go#62278 and dependabot/dependabot-core#7895.

Trying to use 1.21.0 as module version

module github.com/foo/bar

go 1.21.0

fails with:

go.mod:3: invalid go version '1.21.0': must match format 1.23

We have the following internal code style:

• Standard library imports
• repository imports
• Vendor imports
• Named imports (all internal and external named should be in same group)

Example:

import (
	"context"
	"fmt"
	"time"

	"github.com/ourcompany/ourcompany/pkg/log"
	"github.com/ourcompany/ourcompany/pkg/percentile"

        "github.com/google/go-cmp/cmp"

	svc "github.com/ourcompany/ourcompany/pkg/aggregator_service"
)

Could you add support for the final named imports group?

so while using this linter in ci we might need to return error if something is wrong and to not let linter modify file.

For example(local module is github.com/cosmos/cosmos-sdk), I got this after run goimports-reviser:

import (
	"encoding/json"
	"errors"
	"fmt"
	"net/url"
	"os"
	"path/filepath"
	"strconv"
	"strings"
	"time"

	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
	"github.com/rs/zerolog"

	cverrors "github.com/cosmos/cosmos-sdk/cosmovisor/errors"
)

But it should be like this right?:

import (
	"encoding/json"
	"errors"
	"fmt"
	"net/url"
	"os"
	"path/filepath"
	"strconv"
	"strings"
	"time"

	"github.com/rs/zerolog"

	cverrors "github.com/cosmos/cosmos-sdk/cosmovisor/errors"
	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

==> Tapping incu6us/tap
Cloning into '/opt/homebrew/Library/Taps/incu6us/homebrew-tap'...
remote: Enumerating objects: 152, done.
remote: Counting objects: 100% (152/152), done.
remote: Compressing objects: 100% (112/112), done.
remote: Total 152 (delta 36), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (152/152), 19.39 KiB | 461.00 KiB/s, done.
Resolving deltas: 100% (36/36), done.
Error: Invalid formula: /opt/homebrew/Library/Taps/incu6us/homebrew-tap/Formula/goimports-reviser.rb
formulae require at least a URL

My config for goimports-reviser is:

return {
  {
    "jose-elias-alvarez/null-ls.nvim",
    opts = function(_, opts)
      local nls = require("null-ls")
      local nls_sources = {
        -- go
        nls.builtins.formatting.goimports_reviser.with({
          generator_opts = {
            command = "goimports-reviser",
            args = { "-output", "stdout", "$FILENAME" },
            to_stdin = true,
          },
        }),
      }
      vim.list_extend(opts.sources, nls_sources, 1, #nls_sources)
    end,
  },
}

In a very simple go project with go.mod:

module minimal

go 1.19

main.go:

package main

func main() {
	fmt.Println("Hello, World!")
}

Notice the absence of imports statment as well. So there should be a undeclared name: fmt error. If I format the code, the imports are not auto imported.

If I delete the line fmt.Println() dd then hit :w, the line pops back up.

I'm trying to set both -list-diff and -set-exit-status while using ./... as the target, but they don't seem to work. Instead changes are written to all files with no list of files outputted to stdout or with an exit status.

goimports-reviser -list-diff -set-exit-status ./...

I also tried goimports-reviser -list-diff $(find . -type f -name '*.go') which worked with goimports, but it just lists the first file.

Is there a way to set either of these while also targeting all .go files in my repo?

hey, so I noticed that goimports-reviser removes comment directives by using go/ast.go (g *CommentGroup) Text() as it ignores directives in this line

if isDirective(c) {
    // Ignore //go:noinline, //line, and so on.
    continue
}

I managed to fix this issue locally by copy-pasting Text method and changing few lines, but that does not seem like a clean
solution.

bug example:

Input

import (
	"log"

	"github.com/incu6us/goimports-reviser/testdata/innderpkg"

	"bytes"
	"go/ast" //nolint:gomnd

	"github.com/pkg/errors"
)

Expected

import (
	"bytes"
	"go/ast" //nolint:gomnd
	"log"

	"github.com/pkg/errors"

	"github.com/incu6us/goimports-reviser/testdata/innderpkg"
)

Actual

import (
	"bytes"
	"go/ast" //

	"github.com/pkg/errors"

	"github.com/incu6us/goimports-reviser/testdata/innderpkg"
)

$ cat tools.go
// +build tools

package tools

import (
	_ "github.com/incu6us/goimports-reviser"
)

This is the file for go install to keep it from disappearing with go mod tidy.
See below for details

• https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
• https://github.com/go-modules-by-example/index/blob/master/010_tools/README.md

Running this file with the -rm-unused option will result in an error.

$ goimports-reviser -company-prefixes "github.com/xxx" -project-name -rm-unused -imports-order std,company,project,general -file-path tools.go
2022/11/17 10:49:55 internal error: go list gives conflicting information for package github.com/xxx/api [github.com/xxx/api.test]
main.main
	/Users/sion_cojp/work/go/pkg/mod/github.com/incu6us/goimports-reviser/[email protected]/main.go:330
runtime.main
	/Users/sion_cojp/.goenv/versions/1.18.0/src/runtime/proc.go:250
runtime.goexit
	/Users/sion_cojp/.goenv/versions/1.18.0/src/runtime/asm_arm64.s:1259

Is there any way to avoid this error?

For example:
the command:

goimports-reviser -company-prefixes my_company -rm-unused -output stdout  -imports-order std,company,project,general -format ${file}

code:

package main
import (
	"fmt"

	"my_company"
	
        "my_project"
)

expire

package main
import (
	"fmt"

	"my_company"
        "my_project"
)

Hi there.

We have current tooling that can detect which files were changed in a large repository. We currently pass all the paths to these files to gofmt for formatting, but would like to use this tool instead.

However, since we are only able to pass a single file at a time, there is a performance overhead, as we need to call the binary many times.

If we pass a full folder, the tool will use wasted cycles running on code that was not changed, which will not work on our large codebase.

Is it possible that you could support formatting any number of files given as arguments?

Would be handy at times to be able to only output formatted content to stdout rather than writing it to the file. This is the default behavior for gofmt and goimports. They only write the output if you pass -w.

This is useful when you try to integrate goimports-reviser into a larger toolchain (where another program might want to run goimports-reviser and use the output before writing it).

Would only need to add an extra cmd option and wrap this section in an if statement, more or less 😄 .

I can submit a PR for this (with tests) if you agree on it.

I guess the only question is whether you'd want to conform to the aforementioned formatters and only write if you pass -w (and therefore bump this to v3 since that's a breaking change), or if you just want to add an option for not writing and instead outputting to stdout (non-breaking change). Not sure what the latter option could be called.

MacBook-Pro librdkafka % brew tap incu6us/homebrew-tap
brew install incu6us/homebrew-tap/goimports-reviser
==> Tapping incu6us/tap
Cloning into '/opt/homebrew/Library/Taps/incu6us/homebrew-tap'...
remote: Enumerating objects: 172, done.
remote: Counting objects: 100% (172/172), done.
remote: Compressing objects: 100% (127/127), done.
remote: Total 172 (delta 41), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (172/172), 21.90 KiB | 3.13 MiB/s, done.
Resolving deltas: 100% (41/41), done.
Error: Invalid formula: /opt/homebrew/Library/Taps/incu6us/homebrew-tap/Formula/goimports-reviser.rb
formulae require at least a URL
Error: Cannot tap incu6us/tap: invalid syntax in tap!
==> Tapping incu6us/tap
Cloning into '/opt/homebrew/Library/Taps/incu6us/homebrew-tap'...
remote: Enumerating objects: 172, done.
remote: Counting objects: 100% (172/172), done.
remote: Compressing objects: 100% (127/127), done.
remote: Total 172 (delta 41), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (172/172), 21.90 KiB | 3.13 MiB/s, done.
Resolving deltas: 100% (41/41), done.
Error: Invalid formula: /opt/homebrew/Library/Taps/incu6us/homebrew-tap/Formula/goimports-reviser.rb
formulae require at least a URL
Error: Cannot tap incu6us/tap: invalid syntax in tap!

On Ubuntu I installed goimports-reviser and when I try to run it I get the following error:

/snap/bin/goimports-reviser -file-path /home/ruwan/go/src/project/domain/entities/Config.go
open /home/ruwan/go/src/project/go.mod: permission denied

It would be great to have the ability to pass gofmt options such as -s etc. Looking at https://golang.org/src/cmd/gofmt/ not sure what the best approach would be here. I see goimports-reviser is currently using go/format. Ideally, goimports-reviser wouldn't have to duplicate the work of gofmt, including it's flags, but I also see how calling the gofmt binary may not be desired as well. Happy to make a PR, but would appreciate some feedback since I'm not very familiar with this.

Thanks again for creating this fantastic tool.

There appears to be an issue in files where no imports are declared, it seems that goimports-reviser is removing all comments that reside within the code, an examples as follows:

package main

// OutputDir the output directory where the built version of Authelia is located.
var OutputDir = "dist"

// DockerImageName the official name of Authelia docker image.
var DockerImageName = "authelia/authelia"

// IntermediateDockerImageName local name of the docker image.
var IntermediateDockerImageName = "authelia:dist"

const masterTag = "master"
const stringFalse = "false"
const stringTrue = "true"
const suitePathPrefix = "PathPrefix"
const webDirectory = "web"

Will turn into the following with a pass of goimports-reviser:

package main

var OutputDir = "dist"

var DockerImageName = "authelia/authelia"

var IntermediateDockerImageName = "authelia:dist"

const masterTag = "master"
const stringFalse = "false"
const stringTrue = "true"
const suitePathPrefix = "PathPrefix"
const webDirectory = "web"

Add option to exclude vendor directory when applying this command. Many projects include vendor in source code but do not wish to apply any rules to these vendored packages

Hi,
At first, thanks for this useful package.
When I run this command, I get this error:

goimports-reviser -rm-unused -set-alias -format -recursive ./...

Error:

-: no Go files in \services\catalog_write_service
Failed to fix directory: failed to walk dif: failed to fix: package has an errors

Add a new line between funcs if there are no delimiters. Basicaly, code below

    func...
    func...

should be like

    func...
    [\n]
    func...

goimports-reviser not recognizing the new stdlib imports that were introduced in 1.16

goimports-reviser -file-path ./test.go -rm-unused -set-alias -format
output:

open go.mod: no such file or directory

Usage of goimports-reviser:
  -file-path string
    	File path to fix imports(ex.: ./reviser/reviser.go). Required parameter.
  -format
    	Option will perform additional formatting. Optional parameter.
  -local string
    	Local package prefixes which will be placed after 3rd-party group(if defined). Values should be comma-separated. Optional parameters.
  -output string
    	Can be "file" or "stdout". Whether to write the formatted content back to the file or to stdout. Optional parameter. (default "file")
  -project-name string
    	Your project name(ex.: github.com/incu6us/goimports-reviser). Optional parameter.
  -rm-unused
    	Remove unused imports. Optional parameter.
  -set-alias
    	Set alias for versioned package names, like 'github.com/go-pg/pg/v9'. In this case import will be set as 'pg "github.com/go-pg/pg/v9"'. Optional parameter.
  -version
    	Show version.

When working on monorepos, it's not uncommon to reference your own project using both the external and the local path.

E.g.:

import (
	"context"

	"github.com/3rdparty/dep"

	"coderepo.com/myproject/lib/versioned/pkg"
	"coderepo.com/myproject/lib/alsoversioned"

	"myproject/foo/bar"
	"myproject/foo/x"
)

Right now it's only possible to specify a single project-name. It would be neat if you could either specify both local and ext:

goimports-reviser -project-name foo -project-name-ext coderepo.com/myproject  -file-path $FilePath$

Or if you could generalize that parameter and make it something like this:

goimports-reviser -import-groups coderepo.com/myproject foo  -file-path $FilePath$

(which would order imports like this: std | 3rd-party | coderepo.com/myproject | foo)

or perhaps:

goimports-reviser -project-name foo -import-groups coderepo.com/myproject  -file-path $FilePath$

(which would order imports like this: std | 3rd-party | <import groups in the order specified> | foo)

What do you think?

What's achievable today is this:

import (
	"context"

	"coderepo.com/myproject/lib/versioned/pkg"
	"coderepo.com/myproject/lib/alsoversioned"
	"github.com/3rdparty/dep"

	"myproject/foo/bar"
	"myproject/foo/x"
)

Which is ok but not ideal.

A range of the form:
for x, _ = range v {...}
should be simplified to:
for x = range v {...}

The current installation docs use brew/snap to install the binary. This is inconvenient for go developers and I would like to simply run go get github.com/incu6us/[email protected]. However, this works only for v1 which is currently 0.6.1.

Fixing this would require moving the current code into v2 dir (which follows go modules practices)

Currently we loop through all go files, grep to see if "^// Code generated .* DO NOT EDIT\.$" matches, and skip those files when applying goimports-reviser. An option to avoid these files would be preferable!

• os

m1 mac macos 13.2

• go version

go version go1.21.0 darwin/arm64

• exec command

goimports-reviser -rm-unused ./...

• command return

Failed to fix directory: path is not a directory

when cgo is enabled, the "C" package should always be imported after the c include comment like this:

/*
#cgo CFLAGS: -I
#cgo LDFLAGS: -L
#include <stdio.h>
#include <stdlib.h>
*/
import "C"
import (
	"reflect"
	"runtime"
	"unsafe"
)

However,
with -rm-unused , "C" will be removed.
without this flag, "C" import will be re-ordered, this operation results in compile failure

import (
	"fmt"

	"C"
)

Or perhaps, if -project-name isn't specified, try to automatically determine it.

For modules (which I assume most people are using now), you just run go list -m anywhere inside that module (so it should be safe to run it in the same dir as the file you're trying to fix).

Might be possible to run it programmatically using something like https://godoc.org/golang.org/x/tools/go/packages as well.

Hi there love the tool! Works pretty much exactly as expected.

golint expects blank imports to have a comment justifying them which have to exist directly above the import. Currently goimports-reviser moves those to the wrong location. They should remain above the import (seems like into a line of their own).

Edit: My mistake, they can exist directly after them on the same line as well. Still an issue but this may be easier to solve!

Running

cat client.go | goimports-reviser -rm-unused -format -output stdout

yields

2022/11/22 10:25:35 open /my/cool/service/<standard-input>: no such file or directory

Since updating to Go 1.16 a number of stdlib packages appear to be sorted in the wrong groups.

The following expected example:

import (
	"embed"
	"io/fs"
	"io/ioutil"
	"net"
	"net/http"
	"os"
	"runtime"
	"strconv"
	"strings"

	duoapi "github.com/duosecurity/duo_api_golang"
	"github.com/fasthttp/router"
	"github.com/valyala/fasthttp"
	"github.com/valyala/fasthttp/expvarhandler"
	"github.com/valyala/fasthttp/fasthttpadaptor"
	"github.com/valyala/fasthttp/pprofhandler"

	"github.com/authelia/authelia/internal/configuration/schema"
	"github.com/authelia/authelia/internal/duo"
	"github.com/authelia/authelia/internal/handlers"
	"github.com/authelia/authelia/internal/logging"
	"github.com/authelia/authelia/internal/middlewares"
)

Is sorted into the following incorrect example:

import (
	"io/ioutil"
	"net"
	"net/http"
	"os"
	"runtime"
	"strconv"
	"strings"

	"embed"
	duoapi "github.com/duosecurity/duo_api_golang"
	"github.com/fasthttp/router"
	"github.com/valyala/fasthttp"
	"github.com/valyala/fasthttp/expvarhandler"
	"github.com/valyala/fasthttp/fasthttpadaptor"
	"github.com/valyala/fasthttp/pprofhandler"
	"io/fs"

	"github.com/authelia/authelia/internal/configuration/schema"
	"github.com/authelia/authelia/internal/duo"
	"github.com/authelia/authelia/internal/handlers"
	"github.com/authelia/authelia/internal/logging"
	"github.com/authelia/authelia/internal/middlewares"
)

When using golang with cgo, we have a preamble with C headers, followed by import "C", and then our go imports, ie

/*
#include <stdlib.h>
*/
import "C"

import (
	"errors"
	"fmt"
)

After running goimports-reviser, the import "C" is not preserved in its current position, which is required in order to use cgo

Am I missing something?

Could you add an excluded folder option?

Been using goimports-reviser a lot since you added support for local, and it's been great. However, we did notice an issue that (I think) mainly applies to when you're working with monorepos.

Scenario
You have a monorepo which lives at myproject.com/code/.
One of the packages (thispkg in this example) refer to one of the other packages.

Current outcome

goimports-reviser -project-name myproject.com/code/thispkg -local myproject.com/code -file-path $FilePath$

context

github.com/3rdparty/pkg

myproject.com/code/foopkg
myproject.com/code/otherpkg
myproject.com/code/thispkg/stuff
myproject.com/code/thispkg/morestuff

Desired outcome

context

github.com/3rdparty/pkg

myproject.com/code/foopkg
myproject.com/code/otherpkg

myproject.com/code/thispkg/stuff
myproject.com/code/thispkg/morestuff

Potential Solution
Currently, local is checked before project. If those two are switched around, that should solve this problem and I don't think it would affect anything else. However, I'm open to other ideas/thoughts. Maybe we're doing something unorthodox with our package structure?

A range of the form:
for _ = range v {...}
should be simplified to:
for range v {...}

Trying to install based off of the instructions in the readme, i.e.
brew tap incu6us/homebrew-tap brew install incu6us/homebrew-tap/goimports-reviser

When I run brew tap, I get the following:

brew tap incu6us/homebrew-tap
==> Tapping incu6us/tap
Cloning into '/usr/local/Homebrew/Library/Taps/incu6us/homebrew-tap'...
remote: Enumerating objects: 115, done.
remote: Counting objects: 100% (115/115), done.
remote: Compressing objects: 100% (84/84), done.
remote: Total 115 (delta 27), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (115/115), 14.32 KiB | 3.58 MiB/s, done.
Resolving deltas: 100% (27/27), done.
Error: Invalid formula: /usr/local/Homebrew/Library/Taps/incu6us/homebrew-tap/Formula/ckafka.rb
formulae require at least a URL
Error: Cannot tap incu6us/tap: invalid syntax in tap!

HomeBrew Version: 2.5.11

An array, slice, or map composite literal of the form:
[]T{T{}, T{}}
should be simplified to:
[]T{{}, {}}

For today the tool can't remove code block with empty imports like import(). Good to have it.

A slice expression of the form:
s[a:len(s)]
should be simplified to:
s[a:]

Hello,

As always thanks for providing this awesome tool!

Issue
If a go file has multiple imports go-imports-reviser does not properly handle that scenario. e.g

import "sync"
import "testing"

// yolo
import "fmt"


// not sure why this is here but we shall find out soon enough
import "io"


func TestMe(t *testing.T){
	var mu sync.RWMutex
	_ = io.ReadFull

	fmt.Println("yo")
	mu.Lock()
	defer mu.Unlock()
}

becomes

import (
	"fmt"
	"io"
	"sync"
	"testing"
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
	// yolo
)
import (
	"fmt"
	"io"
	"sync"
	"testing"
)

import (
	"fmt"
	"io"
	"sync"

	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	// not sure why this is here but we shall find out soon enough
	"testing"
)

import (
	"fmt"
	"io"
	"sync"
	"testing"
)

func TestMe(t *testing.T) {
	var mu sync.RWMutex
	_ = io.ReadFull

	fmt.Println("yo")
	mu.Lock()
	defer mu.Unlock()
}

Context
I have some code generation tools that's producing multiple imports which is how I stumbled on this issue.

Build Info

> goimports-reviser --version
version: 2.3.2
build with: 
tag: v2.3.2
commit: 7691af84d68efe1daf88a064804977332d6ad6da
source: https://github.com/incu6us/goimports-reviser

Unfortunately as of now you cannot use this package against code with generics. As far as I can tell, the package does work well with 1.18 code that does not use generics, but it is possible other features of this version I have not used also break the reviser.

Error trace:

2022/05/31 17:29:01 115:33: expected '(', found '[' (and 10 more errors)
main.main
	/home/runner/work/goimports-reviser/goimports-reviser/main.go:175
runtime.main
	/opt/hostedtoolcache/go/1.14.15/x64/src/runtime/proc.go:203
runtime.goexit
	/opt/hostedtoolcache/go/1.14.15/x64/src/runtime/asm_amd64.s:1373

An example of the code that probably triggered this trace

func EqualableArraysEqual[T Equalable[T]](a1, a2 []T) bool {
        ...
}

It would be nice if the aliased imports . and _ were grouped separately.

Example from gci:

from

package main
import (
  "fmt"
  go "github.com/golang"
  _ "github.com/golang/blank"
  . "github.com/golang/dot"
  "github.com/daixiang0/gci"
  _ "github.com/daixiang0/gci/blank"
  . "github.com/daixiang0/gci/dot"
)

to

package main
import (
  "fmt"

  go "github.com/golang"

  "github.com/daixiang0/gci"

  _ "github.com/daixiang0/gci/blank"
  _ "github.com/golang/blank"

  . "github.com/daixiang0/gci/dot"
  . "github.com/golang/dot"
)