Multiple directories do not sum correctly

[Kerollmops]

Describe the bug
When using scc on multiple directories doesn't sum both directories independently.

To Reproduce

Documents; scc meilisearch
───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust                       372    114690    11683      6453    96554       5299
TOML                        27      1066      134       117      815          2
YAML                        23      1911      158       122     1631          0
Markdown                    15      1347      315         0     1032          0
JSON                        11      4088        0         0     4088          0
SVG                          5       108        0         1      107          0
Shell                        5       308       29        32      247         10
JSONL                        2        79        0         0       79          0
Docker ignore                1         4        0         0        4          0
Dockerfile                   1        48       12         9       27          3
License                      1        21        4         0       17          0
Python                       1        25        6         7       12          4
───────────────────────────────────────────────────────────────────────────────
Total                      464    123695    12341      6741   104613       5318
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $3 565 826
Estimated Schedule Effort (organic) 22,30 months
Estimated People Required (organic) 14,21
───────────────────────────────────────────────────────────────────────────────
Processed 4607051 bytes, 4.607 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────

Documents; scc heed
───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust                        40     10505      944      3770     5791        426
C                           14     14631     1267      2142    11222       2785
TOML                         7       207       21        47      139          1
License                      3        89       19         0       70          0
C Header                     2      1855      120      1467      268          2
Markdown                     2        66       18         0       48          0
Plain Text                   2       135       10         0      125          0
Makefile                     1       119       20        21       78          7
YAML                         1        96        7         1       88          0
───────────────────────────────────────────────────────────────────────────────
Total                       72     27703     2426      7448    17829       3221
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $556 261
Estimated Schedule Effort (organic) 11,01 months
Estimated People Required (organic) 4,49
───────────────────────────────────────────────────────────────────────────────
Processed 926376 bytes, 0.926 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────

Documents; scc meilisearch heed
───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
Rust                        80     21010     1888      7540    11582        852
C                           28     29262     2534      4284    22444       5570
TOML                        14       414       42        94      278          2
License                      6       178       38         0      140          0
C Header                     4      3710      240      2934      536          4
Markdown                     4       132       36         0       96          0
Plain Text                   4       270       20         0      250          0
Makefile                     2       238       40        42      156         14
YAML                         2       192       14         2      176          0
───────────────────────────────────────────────────────────────────────────────
Total                      144     55406     4852     14896    35658       6442
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $1 151 755
Estimated Schedule Effort (organic) 14,51 months
Estimated People Required (organic) 7,05
───────────────────────────────────────────────────────────────────────────────
Processed 1852752 bytes, 1.853 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────

Expected behavior
Giving multiple directories to scc must compute the sum of all the stats from the different directories.

Desktop (please complete the following information):

• OS: macOS
• Version scc version 3.3.0

[boyter]

Well that's embarrassing.

In short I used a new bit of Go functionality inside the code walker https://github.com/boyter/gocodewalker by using errgroup.Group{} without actually realizing that its eg.Go call is non blocking, so it counts in this case the second directory twice. Simple enough fix https://github.com/boyter/gocodewalker/blob/master/file.go#L160 but annoying.

Anyway pushed out 2 releases, the first which fixes that, and the second which fixes the version (small but I wanted it fixed) https://github.com/boyter/scc/releases/tag/v3.3.2

I had a feeling moving over to the new walker would cause an issue somewhere. Thanks for picking it up so quickly.

[Tommy-42]

Hey @boyter

i think the go team fixed that behavior on go 1.22 https://go.dev/blog/loopvar-preview

maybe worth looking into upgrading go to 1.22, let me know what you think, I might have miss understood the underlying problem

[boyter]

@Tommy-42 oddly I am using Go 1.22 to compile it. I thought it would have resolved it as well, but not in this case. Although it could be falling back to older style of loop as its Go 1.20 in the go.mod

[Tommy-42]

@boyter I did few test and it is kinda weird behavior

I removed your fix ( copy var ) from the gocodewolker :

if len(f.directories) != 0 {
	eg := errgroup.Group{}
	for _, directory := range f.directories {
		eg.Go(func() error {
			return f.walkDirectoryRecursive(directory, []gitignore.GitIgnore{}, []gitignore.GitIgnore{})
		})
	}

	err = eg.Wait()
} else {
	if f.directory != "" {
		err = f.walkDirectoryRecursive(f.directory, []gitignore.GitIgnore{}, []gitignore.GitIgnore{})
	}
}

✗ go version
go version go1.22.2 darwin/arm64

I tried : go run main.go ../meilisearch ../zconfig

it didn't sum both directory, I also tried to update the go.mod to 1.22 it didn't work as well.

then I looked at the link I sent you and they say that you can try it out, with :
GOEXPERIMENT=loopvar go run main.go ../meilisearch ../zconfig

and it worked, the sum of both directory was done

I found the proposal : golang/go#60078 it appears to be done and merged on 1.22 so I am not sure whats the underlying problem there

both following links state that it should work
https://tip.golang.org/ref/spec#For_range , https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/loopclosure

I don't know if it is because of the go.mod of the gocodewalker repository that is not go 1.22 and could block the feature even if it is in vendor folder

[boyter]

Odd.... One I will want to revisit that's for sure because I knew I was using Go 1.22 hence not capturing the variable, but as you say perhaps all the go.mod files need to be on 1.22 in order to ensure it works.

Sounds like a great "gotcha" blog post that would get attention though.

[Tommy-42]

Ok I tried that :

I cloned the gocodewalker repo and updated the go.mod to 1.22 and removed the fix.

I modified the go.mod of scc to use local version of gocodewalker

replace github.com/boyter/gocodewalker => /Users/tommy/Codes/github/gocodewalker

I deleted the vendor folder

run go run main.go ../meilisearch ../zconfig

and it worked.

I changed back the go.mod to 1.20 and it failed as expected.

So I guess we have our answer

[boyter]

Well that answers that. Thanks for the investigation.