x/tools/gopls: panic in analyzer due to type checker errors

[muirdm]

Edit: I've narrowed the issue down further, see follow up comment below.

I'm seeing these two separate crashes on master (08e0b30).

2019/06/03 09:36:15 Debug serving on port: 63790
panic: interface conversion: types.Object is nil, not *types.Func

goroutine 63 [running]:
golang.org/x/tools/go/analysis/passes/printf.maybePrintfWrapper(0xc0002a80f0, 0x174d0e0, 0xc000284870, 0x0)
	/Users/muir/projects/tools/go/analysis/passes/printf/printf.go:115 +0x30c
golang.org/x/tools/go/analysis/passes/printf.findPrintfLike(0xc00027c3c0, 0x10, 0x15cd100, 0xdf793dbdbb211201, 0xc000288630)
	/Users/muir/projects/tools/go/analysis/passes/printf/printf.go:154 +0xf2
golang.org/x/tools/go/analysis/passes/printf.run(0xc00027c3c0, 0xc000288600, 0xc0000dc270, 0x2, 0x1)
	/Users/muir/projects/tools/go/analysis/passes/printf/printf.go:83 +0x2b
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc000354e40, 0x174ea20, 0xc000282880, 0xc0000f9580, 0xc0002e96c8, 0x10)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:164 +0x79c
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:100 +0x4e
sync.(*Once).Do(0xc000354e40, 0xc0002e9708)
	/usr/local/go/src/sync/once.go:44 +0xb3
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc000354e40, 0x174ea20, 0xc000282880, 0xc0000f9580, 0xc000285178, 0xc0002e9790)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:99 +0x8b
golang.org/x/tools/internal/lsp/source.execAll.func1(0xc000000008, 0x169b7a0)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:91 +0x48
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc000285230, 0xc000280620)
	/Users/muir/projects/XXX/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x57
created by golang.org/x/sync/errgroup.(*Group).Go
	/Users/muir/projects/XXX/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x66

2019/06/03 09:36:41 Debug serving on port: 63793
panic: interface conversion: types.Object is nil, not *types.Func

goroutine 101 [running]:
golang.org/x/tools/go/analysis/passes/ctrlflow.run.func1(0x1745720, 0xc00028c5a0)
	/Users/muir/projects/tools/go/analysis/passes/ctrlflow/ctrlflow.go:105 +0x398
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc0001aaf00, 0xc000294c40, 0x2, 0x2, 0xc000294c60)
	/Users/muir/projects/tools/go/ast/inspector/inspector.go:77 +0x9f
golang.org/x/tools/go/analysis/passes/ctrlflow.run(0xc000362320, 0xc0001ac500, 0xc00002a330, 0x2, 0x1)
	/Users/muir/projects/tools/go/analysis/passes/ctrlflow/ctrlflow.go:102 +0x1a1
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc0002dab40, 0x174ea20, 0xc0000c49c0, 0xc0000b8740, 0xc00037bec8, 0x10)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:164 +0x79c
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:100 +0x4e
sync.(*Once).Do(0xc0002dab40, 0xc00037bf08)
	/usr/local/go/src/sync/once.go:44 +0xb3
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc0002dab40, 0x174ea20, 0xc0000c49c0, 0xc0000b8740, 0x0, 0xc00037bf90)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:99 +0x8b
golang.org/x/tools/internal/lsp/source.execAll.func1(0x8, 0x169b7a0)
	/Users/muir/projects/tools/internal/lsp/source/analysis.go:91 +0x48
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc0001a9350, 0xc0001aae20)
	/Users/muir/projects/XXX/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x57
created by golang.org/x/sync/errgroup.(*Group).Go
	/Users/muir/projects/XXX/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x66

I've narrowed it down to this repro involving build tags:

$ find foo
foo
foo/p2
foo/p2/p2.go
foo/go.mod
foo/p1
foo/p1/p1.go
foo/p1/p1_12.go

$ cat foo/go.mod
module foo

go 1.12

$ cat foo/p1/p1.go
// +build !go1.12

package p1

func Foo() {
}

$ cat foo/p1/p1_12.go
// +build go1.12

package p1

func Foo() {
}

$ cat foo/p2/p2.go
package p2

import "foo/p1"

func foo() {
	p1.Foo()
}

To reproduce:

1. open p1.go (I'm using go1.12, so this file doesn't apply to my build)
2. open p2.go, witness panic

I tried in VSCode and it didn't seem to reproduce as easily, so here is me Emacs lsp debug log:

[Trace - 09:57:37 AM]
Sending request 'initialize - (7465)'.

Params: 
{
  "processId": 11690,
  "rootPath": "/Users/muir/scratch/foo/",
  "rootUri": "file:///Users/muir/scratch/foo/",
  "capabilities": {
    "workspace": {
      "workspaceEdit": {
        "documentChanges": true,
        "resourceOperations": [
          "create",
          "rename",
          "delete"
        ]
      },
      "applyEdit": true,
      "symbol": {
        "symbolKind": {
          "valueSet": [
            1,
            2,
            3,
            4,
            5,
            6,
            7,
            8,
            9,
            10,
            11,
            12,
            13,
            14,
            15,
            16,
            17,
            18,
            19,
            20,
            21,
            22,
            23,
            24,
            25,
            26
          ]
        }
      },
      "executeCommand": {
        "dynamicRegistration": false
      },
      "didChangeWatchedFiles": {
        "dynamicRegistration": true
      },
      "workspaceFolders": true,
      "configuration": true
    },
    "textDocument": {
      "declaration": {
        "linkSupport": true
      },
      "definition": {
        "linkSupport": true
      },
      "implementation": {
        "linkSupport": true
      },
      "typeDefinition": {
        "linkSupport": true
      },
      "synchronization": {
        "willSave": true,
        "didSave": true,
        "willSaveWaitUntil": true
      },
      "documentSymbol": {
        "symbolKind": {
          "valueSet": [
            1,
            2,
            3,
            4,
            5,
            6,
            7,
            8,
            9,
            10,
            11,
            12,
            13,
            14,
            15,
            16,
            17,
            18,
            19,
            20,
            21,
            22,
            23,
            24,
            25,
            26
          ]
        },
        "hierarchicalDocumentSymbolSupport": true
      },
      "formatting": {
        "dynamicRegistration": true
      },
      "codeAction": {
        "dynamicRegistration": true,
        "codeActionLiteralSupport": {
          "codeActionKind": {
            "valueSet": [
              "",
              "quickfix",
              "refactor",
              "refactor.extract",
              "refactor.inline",
              "refactor.rewrite",
              "source",
              "source.organizeImports"
            ]
          }
        }
      },
      "completion": {
        "completionItem": {
          "snippetSupport": true
        },
        "contextSupport": true
      },
      "signatureHelp": {
        "signatureInformation": {
          "parameterInformation": {
            "labelOffsetSupport": true
          }
        }
      },
      "documentLink": {
        "dynamicRegistration": true
      },
      "hover": {
        "contentFormat": [
          "plaintext",
          "markdown"
        ]
      },
      "foldingRange": {
        "dynamicRegistration": true,
        "rangeLimit": null,
        "lineFoldingOnly": null
      }
    }
  },
  "initializationOptions": null
}




[Trace - 09:57:37 AM]
Received response 'initialize - (7465)' in 31ms.

Result: 
{
  "custom": null,
  "capabilities": {
    "workspace": {
      "workspaceFolders": {
        "changeNotifications": "workspace/didChangeWorkspaceFolders",
        "supported": true
      }
    },
    "typeDefinitionProvider": true,
    "documentLinkProvider": {
    },
    "documentFormattingProvider": true,
    "codeActionProvider": true,
    "documentSymbolProvider": true,
    "documentHighlightProvider": true,
    "definitionProvider": true,
    "signatureHelpProvider": {
      "triggerCharacters": [
        "(",
        ","
      ]
    },
    "completionProvider": {
      "triggerCharacters": [
        "."
      ]
    },
    "hoverProvider": true,
    "textDocumentSync": {
      "change": 1,
      "openClose": true
    }
  }
}




[Trace - 09:57:37 AM]
Sending notification 'initialized'.

Params: 
{
}




[Trace - 09:57:37 AM]
Sending notification 'textDocument/didOpen'.

Params: 
{
  "textDocument": {
    "uri": "file:///Users/muir/scratch/foo/p1/p1.go",
    "languageId": "go",
    "version": 27,
    "text": "// +build !go1.12\n\npackage p1\n\nfunc Foo() {\n}\n"
  }
}




[Trace - 09:57:37 AM]
Received request 'workspace/configuration - (1).

Params: 
{
  "items": [
    {
      "section": "gopls",
      "scopeUri": "file:///Users/muir/scratch/foo/"
    }
  ]
}




[Trace - 09:57:37 AM]
Sending response 'workspace/configuration - (1)'. Processing request took 0ms

Params: 
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "usePlaceholders": true
    }
  ]
}




[Trace - 09:57:37 AM]
Received notification 'window/logMessage'.

Params: 
{
  "message": "Build info\n----------\ngolang.org/x/tools/cmd/gopls\n    golang.org/x/tools@(devel)\n    golang.org/x/[email protected] h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=\n\nGo info\n-------\ngo version go1.12.1 darwin/amd64\n\nGOARCH=\"amd64\"\nGOBIN=\"\"\nGOCACHE=\"/Users/muir/Library/Caches/go-build\"\nGOEXE=\"\"\nGOFLAGS=\"\"\nGOHOSTARCH=\"amd64\"\nGOHOSTOS=\"darwin\"\nGOOS=\"darwin\"\nGOPATH=\"/Users/muir/projects/XXX/go\"\nGOPROXY=\"\"\nGORACE=\"\"\nGOROOT=\"/usr/local/go\"\nGOTMPDIR=\"\"\nGOTOOLDIR=\"/usr/local/go/pkg/tool/darwin_amd64\"\nGCCGO=\"gccgo\"\nCC=\"clang\"\nCXX=\"clang++\"\nCGO_ENABLED=\"1\"\nGOMOD=\"/Users/muir/scratch/foo/go.mod\"\nCGO_CFLAGS=\"-g -O2\"\nCGO_CPPFLAGS=\"\"\nCGO_CXXFLAGS=\"-g -O2\"\nCGO_FFLAGS=\"-g -O2\"\nCGO_LDFLAGS=\"-g -O2\"\nPKG_CONFIG=\"pkg-config\"\nGOGCCFLAGS=\"-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/96/wknz1jg92yl623qgplk6qw2h0000gn/T/go-build832593399=/tmp/go-build -gno-record-gcc-switches -fno-common\"\n",
  "type": 3
}




[Trace - 09:57:37 AM]
Received notification 'textDocument/publishDiagnostics'.

Params: 
{
  "diagnostics": [
    {
      "message": "other declaration of Foo",
      "source": "LSP",
      "severity": 1,
      "range": {
        "end": {
          "character": 8,
          "line": 4
        },
        "start": {
          "character": 5,
          "line": 4
        }
      }
    }
  ],
  "uri": "file:///Users/muir/scratch/foo/p1/p1_12.go"
}




[Trace - 09:57:37 AM]
Received notification 'textDocument/publishDiagnostics'.

Params: 
{
  "diagnostics": [
    {
      "message": "Foo redeclared in this block",
      "source": "LSP",
      "severity": 1,
      "range": {
        "end": {
          "character": 8,
          "line": 4
        },
        "start": {
          "character": 5,
          "line": 4
        }
      }
    }
  ],
  "uri": "file:///Users/muir/scratch/foo/p1/p1.go"
}




[Trace - 09:57:42 AM]
Sending notification 'textDocument/didOpen'.

Params: 
{
  "textDocument": {
    "uri": "file:///Users/muir/scratch/foo/p2/p2.go",
    "languageId": "go",
    "version": 69,
    "text": "package p2\n\nimport \"foo/p1\"\n\nfunc foo() {\n\tp1.Foo()\n}\n"
  }
}

[muirdm]

I've found the panics are not directly related to build tags. They happen when you have type checker error like having two functions of the same name. The crash is reproducible by defining the function "Foo" twice in one package, then opening another package that imports the first package.

In particular, one of the "Foo" methods has no type information since it is not valid, and some of the analyzers assume that *ast.FuncDecl nodes have type information.

The panic seems to have started after this change golang/tools@08bd53a. @stamblerre do you think this is a gopls bug and it shouldn't be invoking the analyzers, or should the analyzers not crash in this case?

[ianthehat]

No analyzer that sets RunDespiteErrors should crash on things that did not type check, so I would say this is a problem with the analyzer.

[muirdm]

The two analyzers in question don't set RunDespiteErrors. gopls used to define "errors" as len(act.Pkg.GetErrors()) > 0 but now defines it as act.Pkg.IsIllTyped() which is much laxer and causes it to get into these analyzers even with some type checker errors.

[stamblerre]

Ah, I think my reasoning here was to sort of hide the problems caused by build tags, but of course it always comes back to build tags. Basically, you get false errors from packages with build tags, and then any package that imports those can't run analyzers because there are no results from the dependency packages. I guess the real solution here is to figure out how to handle build tags...

[stamblerre]

Duplicate of #33689