Rework module detection for golangci-lint integration

[F0rzend]

What version of GCI are you using?

v0.13.1

Problem

Golangci-lint cannot use latest version of the gci.

Reason described in the comment

[CarlosRDomin]

@daixiang0 do you have an intuition of how easy/difficult it would be to rewrite the localmodule logic to use https://github.com/golangci/modinfo as suggested? I'd be happy to help make this great feature land on golangci-lint, but I'm not familiar with neither modinfo nor gci, so all help would be welcome

[daixiang0]

@CarlosRDomin not hard to do it, you can add modinfo in https://github.com/daixiang0/gci/blob/master/pkg/analyzer/analyzer.go#L44, then you can get mod file obj in the https://github.com/daixiang0/gci/blob/master/pkg/analyzer/analyzer.go#L50.

[CarlosRDomin]

Apologies for the delay, I've been fairly busy and just had a bit of time to take a look into the issue.

Based on this comment, the issue is with findLocalModule directly reading go.mod, not in the analyzer itself. I unfortunately would need @ldez's expertise here (or @daixiang0's), I've never use Golang's analysis package but based on the docs I can only find a way to run the static analysis through singlechecker.Main, which doesn't work since that executes the analyzer as the main command and exits, but instead the current code expects the findLocalModule to retrieve the package it is being called from and continue with the rest of the gci business logic.

I don't have enough knowledge to make gci runnable inside golangci-lint without any further help. @ldez, out of curiosity, have you already tried running the latest gci through golangci-lint and you saw errors or what made you think that the current implementation wouldn't work? Do you have a failing test I could run with a debugger and try to fix? I'd love to help but really don't know how at this point :(

[ldez]

have you already tried running the latest gci through golangci-lint and you saw errors or what made you think that the current implementation wouldn't work?

I don't need to run it to know that just reading a go.mod inside the current dir will never work as expected when you run golangci-lint inside a package.

---

The problem here is still the same: the design.

The elements inside this comment are not helpful because the information about that module requires the pass but the LocalModule needs to access this information here:

gci/pkg/section/local_module.go

Line 38 in 4725b0c

func (m *LocalModule) Configure() error {

gci/pkg/config/config.go

Lines 96 to 106 in 4725b0c

	func configureSections(sections section.SectionList) error {
	for _, sec := range sections {
	switch s := sec.(type) {
	case *section.LocalModule:
	if err := s.Configure(); err != nil {
	return err
	}
	}
	}
	return nil
	}

gci/pkg/config/config.go

Lines 43 to 54 in 4725b0c

	func (g YamlConfig) Parse() (*Config, error) {
	var err error
	sections, err := section.Parse(g.SectionStrings)
	if err != nil {
	return nil, err
	}
	if sections == nil {
	sections = section.DefaultSections()
	}
	if err := configureSections(sections); err != nil {
	return nil, err

And so, to every call to YamlConfig.Parse() but those calls happen before the setup and the run of the analyzer: so it's impossible to access the pass.

[ldez]

After digging more into the code: it's impossible to get something to work as expected without rewriting a large part of the gci code (and not extending it).

@CarlosRDomin, without deeply changing the gci design it's useless to try to fix that inside gci.
So, I will be forced to borrow some pieces of gci code and rewrite them inside golangci-lint, it's not a good sign.

At some point, if the design doesn't evolve in the right direction, either we will have all the code of gci "customized" and inside golangci-lint, or we will have to fork it, or just use another linter.

When 500 LOC becomes more than 3000 LOC to provide the same thing as before, and the usage as a lib is unexpectedly complex, requires a lot of plumbing, and to borrow the code, I think it's just honest to say that there is a major design problem.

[daixiang0]

Now GCI use Analyzer layer to integrate with Golangci-lint like some linters do, I have added this in #86 TODO.

For this issue, please see this.

[ldez]

The "fix" (a kind of copy of my hack) is in the continuity of the current design problems of gci, and fix nothing inside golangci-lint.

The core of the design problem is not fixed, it should be fixed.

[ccoVeille]

The "fix" (a copy of my hack) is in the continuity of the current design problems of gci. The core of the design problem is not fixed and should be fixed.

@ldez does an amazing job ongolangci-lint, and I assume he is a nice guy. But this comment sounded a bit rude to me 😅

I'm sure a smiley would have helped.

Respectfully, a fan of gci and golangci-lint

[ldez]

But this comment sounded a bit rude to me.
...
I'm sure a smiley would have helped.

Because it is, I'm not happy with the situation, so I will not add any smiley.

You are nice, but I don't need help handling this situation the way I want.