Skip to content
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.

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

go/parser: composite lit fields resolve incorrectly #45160

Open
findleyr opened this issue Mar 22, 2021 · 2 comments
Open

go/parser: composite lit fields resolve incorrectly #45160

findleyr opened this issue Mar 22, 2021 · 2 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@findleyr
Copy link
Contributor

Resolving struct fields is a known limitation of go/parser object resolution. However, there is an additional bug that composite lit fields may be incorrectly resolved to unrelated declarations in the package scope. For example:

func _() {
  var tests = []dirLinkTest {
    {
      mklink: func(link, target string) error { return nil },
    },
  }
}

func mklink() {}

The mklink field identifier will be incorrectly resolved to the mklink function (found in os/os_windows_test.go).

Upon investigating this bug, it seems that it is a known limitation:
https://cs.opensource.google/go/go/+/master:src/go/parser/parser.go;l=1610;drc=0bd308ff27822378dc2db77d6dd0ad3c15ed2e08

Will be fixed as part of #45104.

CC @griesemer
@findleyr findleyr added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 22, 2021
@findleyr findleyr added this to the Backlog milestone Mar 22, 2021
@findleyr findleyr self-assigned this Mar 22, 2021
@gopherbot
Copy link

Change https://golang.org/cl/304450 mentions this issue: go/parser: add data-driven tests for object resolution

gopherbot pushed a commit that referenced this issue Mar 30, 2021
@findleyr
go/parser: add data-driven tests for object resolution
0fdd371 
Add new tests for object resolution driven by source files with
declarations and uses marked via special comments. This made it easier
to add test coverage while refactoring object resolution for #45104.

Tests are added to a new resolver_test.go file. In a subsequent CL the
resolver.go file will be added, making this choice of file name more
sensible.

For #45104
For #45136
For #45160

Change-Id: I240fccc0de95aa8f2d03e39c77146d4c61f1ef9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/304450
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/432137 mentions this issue: go/analysis/passes/loopclosure: use object resoluttion from go/types

gopherbot pushed a commit to golang/tools that referenced this issue Sep 21, 2022
@findleyr
go/analysis/passes/loopclosure: use go/types' object resolution
16b9742 
Object resolution in go/types is more accurate than the purely syntactic
object resolution in go/parser (for example, golang/go#45160), and spans
multiple files. Use it for more accurate diagnostics in the loopclosure
analyzer.

Change-Id: I2160876dd4b3fd83aa07a9da971f27c17d7d6043
Reviewed-on: https://go-review.googlesource.com/c/tools/+/432137
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@adonovan @gopherbot @findleyr