x/tools/imports: upper-cased import paths not being selected as candidates #16402

vincepri · 2016-07-17T15:40:20Z

I just noticed that import paths that start with an uppercase letter don't get imported automatically after the update.

Looking at the code and https://github.com/golang/tools/blob/master/imports/fix.go#L651,
I tried to change the behavior, using strings.ToLower on lastTwoComponents and the package gets imported automatically.

What version of Go are you using (go version)?
1.6.2
What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/vincenzo"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.6.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.6.2/libexec/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="0"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?
Wrote a simple test to make goimports find an import path with an uppercase letter

func TestFindUpperCasedImportGoPath(t *testing.T) {
    testConfig{
        gopathFiles: map[string]string{
            "golang.org/bits/Bytes/bytes.go": "package bytes\ntype Buffer2 struct {}\n",
        },
    }.test(t, func(t *goimportTest) {
        got, rename, err := findImportGoPath("bytes", map[string]bool{"Buffer2": true}, filepath.Join(t.gopath, "src/golang.org/x.go"))
        if err != nil {
            t.Fatal(err)
        }
        want := "golang.org/bits/Bytes"
        if got != want || !rename {
            t.Errorf(`findImportGoPath("bytes", Buffer2 ...) = %q, %t; want %q, true`, got, rename, want)
        }
    })
}

What did you expect to see?
Test passing and two candidates for TestFindUpperCasedImportGoPath.

> Environment:
>   GOROOT=/usr/local/Cellar/go/1.6.2/libexec
>   GOPATH=/Users/vincenzo
> Directory: /Users/vincenzo/src/golang.org/x/tools/imports
> Command: /usr/local/bin/go test -v
> Output:
=== RUN   TestFixImports
--- PASS: TestFixImports (0.00s)
=== RUN   TestImportSymlinks
2016/07/17 10:38:12 mypkg candidate 1/1: x/mypkg
--- PASS: TestImportSymlinks (0.01s)
=== RUN   TestFixImportsVendorPackage
--- PASS: TestFixImportsVendorPackage (0.00s)
=== RUN   TestFindImportGoPath
2016/07/17 10:38:12 bytes candidate 1/1: bits/bytes
2016/07/17 10:38:12 bytes candidate 1/1: bits/bytes
--- PASS: TestFindImportGoPath (0.00s)
=== RUN   TestFindImportInternal
2016/07/17 10:38:12 race candidate 1/5: cmd/trace
2016/07/17 10:38:12 race candidate 2/5: runtime/race
2016/07/17 10:38:12 race candidate 3/5: internal/race
2016/07/17 10:38:12 race candidate 4/5: runtime/trace
2016/07/17 10:38:12 race candidate 5/5: internal/trace
2016/07/17 10:38:12 race candidate 1/3: cmd/trace
2016/07/17 10:38:12 race candidate 2/3: runtime/race
2016/07/17 10:38:12 race candidate 3/3: runtime/trace
--- PASS: TestFindImportInternal (0.01s)
=== RUN   TestFindImportRandRead
--- PASS: TestFindImportRandRead (0.00s)
=== RUN   TestFindImportVendor
2016/07/17 10:38:12 hpack candidate 1/1: golang.org/x/net/http2/hpack
--- PASS: TestFindImportVendor (0.00s)
=== RUN   TestFindUpperCasedImportGoPath
2016/07/17 10:38:12 bytes candidate 1/2: bytes
2016/07/17 10:38:12 bytes candidate 2/2: golang.org/bits/Bytes
--- PASS: TestFindUpperCasedImportGoPath (0.01s)
=== RUN   TestProcessVendor
2016/07/17 10:38:12 hpack candidate 1/1: internal/golang.org/x/net/http2/hpack
--- PASS: TestProcessVendor (0.01s)
=== RUN   TestFindImportStdlib
--- PASS: TestFindImportStdlib (0.00s)
=== RUN   TestRenameWhenPackageNameMismatch
--- PASS: TestRenameWhenPackageNameMismatch (0.00s)
=== RUN   TestOptimizationWhenInGoroot
2016/07/17 10:38:12 bar candidate 1/1: foo/bar/v1
--- PASS: TestOptimizationWhenInGoroot (0.01s)
=== RUN   TestIgnoreDocumentationPackage
2016/07/17 10:38:12 foo candidate 1/1: foo
--- PASS: TestIgnoreDocumentationPackage (0.00s)
=== RUN   TestImportPathToNameGoPathParse
--- PASS: TestImportPathToNameGoPathParse (0.00s)
=== RUN   TestIgnoreConfiguration
2016/07/17 10:38:12 pkg candidate 1/1: otherwise-longer-so-worse.example.net/foo/pkg
--- PASS: TestIgnoreConfiguration (0.01s)
PASS
ok      golang.org/x/tools/imports  0.065s
> Elapsed: 1.098s
> Result: Success

What did you see instead?
Test failed (output below) and only 1 candidate for TestFindUpperCasedImportGoPath.

=== RUN   TestFixImports
--- PASS: TestFixImports (0.00s)
=== RUN   TestImportSymlinks
2016/07/17 10:45:35 mypkg candidate 1/1: x/mypkg
--- PASS: TestImportSymlinks (0.01s)
=== RUN   TestFixImportsVendorPackage
--- PASS: TestFixImportsVendorPackage (0.00s)
=== RUN   TestFindImportGoPath
2016/07/17 10:45:35 bytes candidate 1/1: bits/bytes
2016/07/17 10:45:35 bytes candidate 1/1: bits/bytes
--- PASS: TestFindImportGoPath (0.00s)
=== RUN   TestFindImportInternal
2016/07/17 10:45:35 race candidate 1/5: cmd/trace
2016/07/17 10:45:35 race candidate 2/5: runtime/race
2016/07/17 10:45:35 race candidate 3/5: internal/race
2016/07/17 10:45:35 race candidate 4/5: runtime/trace
2016/07/17 10:45:35 race candidate 5/5: internal/trace
2016/07/17 10:45:35 race candidate 1/3: cmd/trace
2016/07/17 10:45:35 race candidate 2/3: runtime/race
2016/07/17 10:45:35 race candidate 3/3: runtime/trace
--- PASS: TestFindImportInternal (0.00s)
=== RUN   TestFindImportRandRead
--- PASS: TestFindImportRandRead (0.00s)
=== RUN   TestFindImportVendor
2016/07/17 10:45:35 hpack candidate 1/1: golang.org/x/net/http2/hpack
--- PASS: TestFindImportVendor (0.00s)
=== RUN   TestFindUpperCasedImportGoPath
2016/07/17 10:45:35 bytes candidate 1/1: bytes
--- FAIL: TestFindUpperCasedImportGoPath (0.01s)
    fix_test.go:1105: findImportGoPath("bytes", Buffer2 ...) = "", false; want "golang.org/bits/Bytes", true
=== RUN   TestProcessVendor
2016/07/17 10:45:35 hpack candidate 1/1: internal/golang.org/x/net/http2/hpack
--- PASS: TestProcessVendor (0.01s)
=== RUN   TestFindImportStdlib
--- PASS: TestFindImportStdlib (0.00s)
=== RUN   TestRenameWhenPackageNameMismatch
2016/07/17 10:45:35 bar candidate 1/1: foo/bar/v1
--- PASS: TestRenameWhenPackageNameMismatch (0.01s)
=== RUN   TestOptimizationWhenInGoroot
--- PASS: TestOptimizationWhenInGoroot (0.01s)
=== RUN   TestIgnoreDocumentationPackage
2016/07/17 10:45:35 foo candidate 1/1: foo
--- PASS: TestIgnoreDocumentationPackage (0.01s)
=== RUN   TestImportPathToNameGoPathParse
--- PASS: TestImportPathToNameGoPathParse (0.00s)
=== RUN   TestIgnoreConfiguration
2016/07/17 10:45:35 pkg candidate 1/1: otherwise-longer-so-worse.example.net/foo/pkg
--- PASS: TestIgnoreConfiguration (0.01s)
FAIL
exit status 1
FAIL    golang.org/x/tools/imports  0.068s
> Elapsed: 1.119s
> Result: Error

The text was updated successfully, but these errors were encountered:

bradfitz · 2016-07-17T15:51:38Z

Sorry, I'm finding it hard to understand this bug report. It contains a lot of unit test code (and using the old goimports test style, which has a lot of boilerplate) and this:

What did you expect to see?
The package imported correctly.

What is "correctly"?

What did you see instead?
No imports were written.

Well, your What did you do? answer was a unit test, so for What did you see instead? I'd expect to see the failure output of your unit test so I can see which error you're reporting.

Can you just explain it without code, or update the code to use testConfig instead, and show the test failure output?

bradfitz · 2016-07-17T15:52:24Z

Also, you write:

Although, this change breaks also other packages named like go-sync that have a package actually named gosync (like http://github.com/Redundancy/go-sync)

If that's a separate unrelated bug, please file a separate bug. It's easier to track two bugs separately so they can be closed independently.

vincepri · 2016-07-17T17:48:15Z

Updated.
Rewrote the test using testConfig and pasted the output of the test when passing and when not.

bradfitz · 2016-07-17T18:27:25Z

Thanks. That's much better & I now understand.

Normally we don't edit comments, but in this case it makes sense. If you have any future comments, please amend this thread rather than edits, though.

I now also see why a directory named "go-foo" might contain "gofoo" and how that comment is related to this issue. We can address both with this bug.

gopherbot · 2016-07-18T23:00:34Z

CL https://golang.org/cl/25030 mentions this issue.

bradfitz changed the title ~~x/imports: Upper-cased import paths not being selected as candidates~~ x/tools/imports: upper-cased import paths not being selected as candidates Jul 17, 2016

bradfitz self-assigned this Jul 17, 2016

bradfitz added this to the Unreleased milestone Jul 17, 2016

gopherbot closed this as completed in golang/tools@caebc7a Jul 18, 2016

bradfitz mentioned this issue Jan 31, 2017

x/tools/cmd/goimports: does not associate package name with import paths containing dashes #17546

Closed

golang locked and limited conversation to collaborators Jul 19, 2017

gopherbot added the FrozenDueToAge label Jul 19, 2017

rsc unassigned bradfitz Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/imports: upper-cased import paths not being selected as candidates #16402

x/tools/imports: upper-cased import paths not being selected as candidates #16402

vincepri commented Jul 17, 2016 •

edited

bradfitz commented Jul 17, 2016

bradfitz commented Jul 17, 2016

vincepri commented Jul 17, 2016

bradfitz commented Jul 17, 2016

gopherbot commented Jul 18, 2016

x/tools/imports: upper-cased import paths not being selected as candidates #16402

x/tools/imports: upper-cased import paths not being selected as candidates #16402

Comments

vincepri commented Jul 17, 2016 • edited

bradfitz commented Jul 17, 2016

bradfitz commented Jul 17, 2016

vincepri commented Jul 17, 2016

bradfitz commented Jul 17, 2016

gopherbot commented Jul 18, 2016

vincepri commented Jul 17, 2016 •

edited