Skip to content

text/scanner: SkipComments is ignored without ScanComments #71133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
c4rlo opened this issue Jan 5, 2025 · 4 comments
Closed

text/scanner: SkipComments is ignored without ScanComments #71133

c4rlo opened this issue Jan 5, 2025 · 4 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.25

Comments

@c4rlo
Copy link

c4rlo commented Jan 5, 2025 •
edited
Loading

Go version

go1.23.4

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/carlo/.cache/go-build'
GOENV='/home/carlo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/carlo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/carlo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.4'
GODEBUG=''
GOTELEMETRY='off'
GOTELEMETRYDIR='/home/carlo/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/carlo/src/go-scanner-bug/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build186288891=/tmp/go-build -gno-record-gcc-switches'

What did you do?

The text/scanner docs say:

Predefined mode bits to control recognition of tokens. For instance, to configure a Scanner such that it only recognizes (Go) identifiers, integers, and skips comments, set the Scanner's Mode field to:

ScanIdents | ScanInts | SkipComments

With the exceptions of comments, which are skipped if SkipComments is set, unrecognized tokens are not ignored.

However, it seems that SkipComments only works as advertised when combined with ScanComments, despite the example clearly implying otherwise.

The following code demonstrates (see go.dev.play link):

package main

import (
	"fmt"
	"strings"
	"text/scanner"
)

func main() {
	testScanner(scanner.ScanIdents|scanner.ScanInts|scanner.SkipComments, "// comment")
	testScanner(scanner.ScanIdents|scanner.ScanInts|scanner.ScanComments|scanner.SkipComments, "// comment")
}

func testScanner(mode uint, input string) {
	var sc scanner.Scanner
	sc.Init(strings.NewReader(input))
	sc.Mode = mode
	for sc.Peek() != scanner.EOF {
		tok := sc.Scan()
		fmt.Printf("[%s:'%s'] ", scanner.TokenString(tok), sc.TokenText())
	}
	fmt.Println()
}

What did you see happen?

The output is:

["/":'/'] ["/":'/'] [Ident:'comment'] 
[EOF:'']

What did you expect to see?

I would expect the output to be:

[EOF:''] 
[EOF:'']
@ianlancetaylor
Copy link
Member

CC @griesemer

@gabyhelp
Copy link

gabyhelp commented Jan 5, 2025

Related Issues

Related Code Changes

Related Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gabyhelp gabyhelp added the Bug label Jan 6, 2025
@seankhliao seankhliao added Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 6, 2025
@seankhliao
Copy link
Member

I think this is just documentation, the definition of SkipComments notes the requirement to also set ScanComments

SkipComments = 1 << -skipComment // if set with ScanComments, comments become white space

@jba jba added BugReport Issues describing a possible bug in the Go implementation. and removed Bug labels Jan 10, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/646036 mentions this issue: text/scanner: add required ScanComments in example

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 3, 2025
@dmitshur dmitshur added this to the Go1.25 milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BugReport Issues describing a possible bug in the Go implementation. Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.25
Development

No branches or pull requests
7 participants
@c4rlo @dmitshur @ianlancetaylor @gopherbot @seankhliao @jba @gabyhelp