Weather Forecast exercise rejects inline comments

Programming Go
,
1

The current test code in weather_forecast_test.go fails to correctly parse inline comments for the CurrentCondition and CurrentLocation variables. It wrongly asserts that the comments are empty and therefore fails the tests. Example:

var (
    CurrentCondition string // CurrentCondition describes the current weather condition.
    CurrentLocation string // CurrentLocation is the location where the forecast is being made.
)
2

It looks like this issue may be related: go/ast: Free-floating comments are single-biggest issue when manipulating the AST · Issue #20744 · golang/go · GitHub

So trailing comments may be “free-floating” and not directly associated with the neighboring variable declaration node but instead indicated positionally, so will require more complicated parsing to handle correctly.

Maybe this test is being too ambitious given the current state of the language reflection tooling.

3

Hmm. This almost works, but contrary to the documentation in the ast package the Text attribute has not stripped the leading // from the comment:

identifierComment := node.Comment.List[0].Text

Looks like there are bugs in the parser code as of go 1.24.5.

4

This code seems to work. Can I open a pull request?

func testBlockIdentifierComment(t *testing.T, node *ast.ValueSpec) {
	t.Helper()

	identifierName := node.Names[0].Name
	if node.Doc == nil && len(node.Comment.List) == 0 {
		t.Errorf("Exported identifier %s should have a comment", identifierName)
	}

	identifierComment := node.Doc.Text()
	if identifierComment == "" {
		identifierComment = strings.TrimPrefix(node.Comment.List[0].Text, "//")
	}
	want := identifierName

	if ok, errStr := testComment("Variable", identifierName, identifierComment, want); !ok {
		t.Error(errStr)
	}
}
5

cc: @andrerfcsantos

6

After inspecting the Go source code and tests, it seems that it’s not a bug, just a confusing inconsistency in the API. A CommentGroup.Text has the comment markers removed while each individual Comment.Text does not.

7

Have you had a chance to look at this yet? Thanks.

8

I am not on the go maintenance team. Perhaps you meant to ask @andrerfcsantos? :slightly_smiling_face:

He’s pretty busy (as are the other members of the team), so it could take a bit before things get looked at/discussed.

9

Note: I’ve moved this into the Go category

1 Like
10

apologies! I didn’t clock that it wasn’t there already. Thank you for moving it. :blue_heart:

1 Like
11

I think the exercise should give an error when making inline comments.

The reason for this is that tools like go doc work best when the comment precedes the declaration and has no lines between them:

The convention is simple: to document a type, variable, constant, function, or even a package, write a regular comment directly preceding its declaration, with no intervening blank line.

from: godoc documentation

An inline comment would not be picked by tools like go doc, so it should not be valid.

Maybe the improvement here is to detect when a comment is inline and ask the student to make a comment before the declaration.