exercism/go-analyzer

Exercism's Go Analyzer

This is Exercism's automated analyzer for the Go track.

Executing the Analyzer

The analyser takes two parameters:

• the exercise slug, e.g. two-fer
• the path to the solution to analize

Example to execute with binary:

analyze two-fer ~/solution-238382y7sds7fsadfasj23j/

From source with Go installed:

go run ./main.go two-fer ~/solution-238382y7sds7fsadfasj23j/

Build Executable

This will create an executable called analyze.

go generate .
go build -tags build -o analyze .

go generate is called before the build to incorporate all necessary files within the binary.

Docker

To build execute the following from the repositories root directory:

docker build -t exercism/go-analyzer .

To run from docker pass in the solutions path as a volume and execute with the necessary parameters:

docker run -v $(PATH_TO_SOLUTION):/solution exercism/go-analyzer ${SLUG} /solution

Example:

docker run -v ~/solution-238382y7sds7fsadfasj23j:/solution exercism/go-analyzer two-fer /solution

Stats

Twofer

Out of 500 real world solutions we get:

approve           34
disapprove        463
refer_to_mentor   3
ejected (failed)  0

go-analyzer's Issues

docs: clarify suggestions for Difference of Squares

The intent with this issue is to facilitate a discussion about proposed changes to the Difference of Squares mentor suggestion output. (A) pull request(s) can be submitted and linked to this issue to retain historical context.

This question was asked in the #track-go channel: https://exercism-team.slack.com/archives/CAS0EK88H/p1558543383014700

...and answered/discussed further here: https://exercism-team.slack.com/archives/CAS0EK88H/p1558543810015800

At the moment, the proposals for changes to the copy are:

These formulas are not as straightforward as they seem, and you aren't expected to create an efficient solution on your own. We encourage you to ask questions and do a little research. Finding the best algorithm is a key skill in software engineering.

and:

A loop or two will get this done brute force, but you can do better. Try looking at: _ ...and then have a couple of links to math pages.

and:

These formulas are absolutely as straightforward as they seem.

I would propose using these formulas are not obvious rather than these formulas are not as straightforward as they seem. While these algorithms may seem simple and elegant, many students (including myself) will not know/remember that they exist until prompted by a mentor to try searching for them:

func SquareOfSum(n int) int {
    sum := n * (n + 1) / 2
    return sum * sum
}

func SumOfSquares(n int) int {
    return (n * (n + 1) * (2*n + 1)) / 6
}

Use gocritic to check variable names more closely

It does stuff like this:

devicelist/timingdevicelist.go:15:35: captLocal: `Links' should not be capitalized (gocritic)
func GetDeviceList(devices *List, Links *linklist.List, custIDs []int) TimingDeviceAnswer {
                                  ^
devicelist/timingdevicelist.go:254:14: captLocal: `ID' should not be capitalized (gocritic)
func buildID(ID int) string {
             ^

Tests not passing

When running the tests locally with go test, they are failing.

Here's a link to the complete output: https://i.fluffy.cc/43V1dWkmPvGN3SBBbwtbGTX10bkqs7jw.html

A lot of these errors seem to be from golint, which is throwing the message:

"golint": "comments: package comment is detached; there should be no blank lines between it and the package statement

Our tests seem to not expect this message. Maybe this was a change in golint?

We probably should also consider alternatives to golint, as it has been deprecated (related issue: #16)

Raindrops: analyzer keeps saying variable should be declared & initialized on oneline

In the raindrops exercise, the analyzer gives the following comment, even though the variable is already declared and initialized on one line:

Rather than declaring the variable result first, and then assigning a value to it afterwards, it is more common to declare and assign in a single statement.

This was the code in the iteration:

package raindrops

import (
    "strconv"
)

// Convert number to raindrops string
func Convert(n int) string {
    result := ""

    if n%3 == 0 {
        result += "Pling"
    }
    if n%5 == 0 {
        result += "Plang"
    }
    if n%7 == 0 {
        result += "Plong"
    }
    if len(result) == 0 {
        return strconv.Itoa(n)
    }
    return result
}

I also tried using var result = "" instead, but still got the same comment from the analyzer.

raindrops: panic on for loop without condition

This solution breaks Exalysis, and presumably now also go-analyzer. Here's the problem:

	for {
		...
	}

In examExtensiveForLoop, this code assumes that every for loop will have a condition:

		if loop.Cond().FindFirstByName(paramName) != nil {
			...
		}

In this case, loop.Cond() is nil:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xa0 pc=0x1466eed]

goroutine 1 [running]:
github.com/exercism/exalysis/track/raindrops.examExtensiveForLoop(0xc0000d0140, 0xc0001d20c0)
        /Users/john/git/bitfield/exalysis/track/raindrops/raindrops.go:116 +0x16d

hamming: syntax error

The analyser is dying here: https://exercism.io/mentor/solutions/72286d2cc829439ab0dfb0fb33cc818b — Unfortunately there is a syntax error in this solution. This causes the tests to fail, which can be verified by running the tests in a terminal with go test.`

cannot install exalysis

go get github.com/exercism/go-analyzer
# github.com/exercism/go-analyzer/suggester/sugg
go/src/github.com/exercism/go-analyzer/suggester/sugg/comments.go:40:11: undefined: strings.ReplaceAll
go/src/github.com/exercism/go-analyzer/suggester/sugg/comments.go:86:19: undefined: strings.ReplaceAll

Update to Go 1.18

Go 1.18 is a very anticipated release of the language due to the inclusion of generics, among other things. We should update the tooling of the Go track to support solutions using Go 1.18.

• Go 1.18 release notes

This update should be similar to the upgrade done for 1.17:

• #26

Tracking issue: exercism/go#2129

The master branch will be renamed to main

In line with our new org-wide policy, the master branch of this repo will be renamed to main. All open PRs will be automatically repointed.

GitHub will show you a notification about this when you look at this repo after renaming:

In case it doesn't, this is the command it suggests:

git branch -m master main
git fetch origin
git branch -u origin/main main

You may like to update the primary branch on your forks too, which you can do under Settings->Branches and clicking the pencil icon on the right-hand-side under Default Branch:

We will post a comment below when this is done. We expect it to happen within the next 12 hours.

[V3] Analyzer extension for concept exercise: basic-slices

This issue describes what the Go analyzer should check for in the card-tricks exercise.

Function GetItem

• no else should be used
• early return should be used which makes a total sum of 2 return statements.
• there needs to be exactly one if statement (with 2 conditions)
• no switch should be used

Function SetItem

• all 4 checks of GetItem apply here as well

Function PrefilledSlice

• check instantiation of the slice. It should be created with a len or cap given
• check for a for loop (no range, not recursive)

Function NumberRow

• check instantiation of the slice. No make or new should be used but a nil slice created with var xy []int
• optional: check if for loop increases index (for i := 1; true; i++)

Function RemoveItemPure

• all 4 checks of GetItem apply here as well
• shouldn't use a for loop, only append

Function RemoveItem

• all 4 checks of GetItem apply here as well

golint: golint is deprecated and frozen

Since golint is unmaintained, deprecated and frozen, maybe we should stop suggesting it and start suggesting alternatives like staticcheck and go vet.

Or maybe refer to IDE integrations, like we do with gofmt.

Relevant issue: golang/go#38968

golint: remove paths from output

Merging of approval states

As part of deploying the comments functionality, we're merging approve_as_optimal and approve_with_comment, and renaming disapprove_with_comment to disapprove. exercism/automated-analysis#56

approve_as_optimal will continued to be honoured until the end of July.

Adhere to comments CI guidelines

https://github.com/exercism/automated-mentoring-support/issues/51