kanstantsin-chernik commented 1 month ago

There is a problem we noticed with formatting after upgrading to v0.2.747:

Some lines are duplicated with and without an alias

To Reproduce To reproduce you need multiple lines with hyphen in the folder name but package name without like this import "someproject/components/standard-metadata"

someproject/components/standard-metadata/some.go file has a standardmetadata package name. In go it is ok to import a package with hyphens without an alias. templ fmt . adds the alias, which isn't a big deal, but breaking imports is.

Expected behavior Either leave imports as is without adding the aliases or add the alias but not remove or duplicate lines

Logs N/A

templ info output Run templ info and include the output.

 % templ info
usage: templ <command> [<args>...]

templ - build HTML UIs with Go

See docs at https://templ.guide

commands:
  generate   Generates Go code from templ files
  fmt        Formats templ files
  lsp        Starts a language server for templ files
  version    Prints the version

% templ version
v0.2.747

Desktop (please complete the following information):

OS: Linux
templ CLI version (templ version): v0.2.747
- Go version (go version): go1.22.5
- gopls version (gopls version): v0.14.2

Additional context Reproducing seems to be deterministic. Same input generates same broken output

kanstantsin-chernik commented 1 month ago

Seems like this PR is at fault

kanstantsin-chernik commented 1 month ago

Here is an example. Input:

package somepackage

import (
    "code.uber.internal/delivery/eats-web-feed/mapper/css-class"
    twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
    tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
    richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
    "code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
    "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
    "thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"
)

templ View(input Input) {
    if input.DisplayType == catalog_section.SomeDisplayType {
        @richtextview.View(sometest.Model{})
    }
    <img class={ cssclass.SomeClasses(), templ.KV("some-class", input.ImageFit == twu.SomeFit) } alt="" { tu.LazyLoad(input.LazyLoadImage)... } src={ tu.ToString("balh") }/>
    @quickaddstepper.View()
    @basetag.View()
}

Output:

package somepackage

import (
    cssclass "code.uber.internal/delivery/eats-web-feed/mapper/css-class"
    twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
    tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
    richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
    basetag "code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
    "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
    quickaddstepper "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
    "thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"
)

templ View(input Input) {
    if input.DisplayType == catalog_section.SomeDisplayType {
        @richtextview.View(sometest.Model{})
    }
    <img class={ cssclass.SomeClasses(), templ.KV("some-class", input.ImageFit == twu.SomeFit) } alt="" { tu.LazyLoad(input.LazyLoadImage)... } src={ tu.ToString("balh") }/>
    @quickaddstepper.View()
    @basetag.View()
}

As you can see, "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper" is duplicated

a-h commented 3 weeks ago

Thanks for the bug report, and sorry for the inconvenience.

The tests for templ auto-import adjustments are in the repo at /cmd/templ/imports/testdata/. Each one is a .txtar file containing something like this.

The first section (delineated by the --) is the input, and the second section is the expected output.

-- fmt_templ.templ --
package test

import (
  sconv "strconv"
)

// Comment on variable or function.
var x = fmt.Sprintf(sconv.Quote("Hello"))
-- fmt_templ.templ --
package test

import (
    "fmt"
    sconv "strconv"
)

// Comment on variable or function.
var x = fmt.Sprintf(sconv.Quote("Hello"))

This makes it pretty easy to add new test cases, so I dropped your sample input into a file, and ran go test in /cmd/templ/imports.

I didn't see a duplicate quick-add-stepper in this test, but I did see css-class and base-tag get stripped out because the packages can't be found by the golang.org/x/tools/imports.Process function. The Process function uses the full filename to find the go.mod file etc. to locate packages.

                package somepackage                                                                                                                                                                                  
                                                                                                                                                                                                                     
                import (                                                                                                                                                                                             
            -           "code.uber.internal/delivery/eats-web-feed/mapper/css-class"
                        twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
                        tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
                        richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
            -           "code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
                        "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
                        "thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"

So, I wonder if the test can't reproduce this because of the lack of go.mod etc.

I'll try that next.

a-h commented 3 weeks ago

Yes, I can reproduce with a simpler example.

go mod init github.com/a-h/templ/cmd/templ/imports/testdata/module
go get github.com/a-h/templ
mkdir css-classes

css-classes/classes.go

package cssclasses

const Header = "header"

main.templ

package main

import (
    "context"
    "github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
    "os"
)

templ View() {
    { cssclasses.Header }
}

func main() {
    View().Render(context.Background(), os.Stdout)
}

Output

This is then formatted to:

package main

import (
    "context"
    "github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
    cssclasses "github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
    "os"
)

templ View() {
    { cssclasses.Header }
}

func main() {
    View().Render(context.Background(), os.Stdout)
}

a-h commented 3 weeks ago

OK, I have a PR for this at https://github.com/a-h/templ/pull/890

If you have time to build templ from source and test it out, that would be great.

a-h commented 2 weeks ago

Actually, might need a bit more thinking as per https://github.com/golang/go/issues/28428 - the expected behaviour is to always add the alias.

But it shouldn't add two copies of the import and break stuff.

a-h commented 2 weeks ago

Fixed in https://github.com/a-h/templ/commit/4b2219c3c39d768c3bea6d997d997835f63ea7e9

kanstantsin-chernik commented 4 days ago

Hi, sorry it took me so long to get back to it. The issue is still there