Fix error when attempting to determine Arduino data path from CLI output -- Needs review, not backwards compatible

[jmcclell]

The arduino-language-server determines the Arduino data directory in one of two ways

1. If the --cli-daemon-addr and --cli-daemon-instance options are set, it uses gRPC to talk to a settings endpoint and extracts the directories.data key.

2. If the --cli-config option is set, it directly execs arduino-cli with arguments config dump --format json and attempts to parse the output as JSON into a pre-defined struct that assumes a particular structure.

With current (and perhaps previous?) versions of Arduino CLI (0.35.3), this second approach generates an error if the config file was generated with arduino-cli config init as the output does not match the code's expected format.

file: inols-err.log (enabled with --log flag on arduino-language-server)

12:13:07.801837 �[INIT --- : running: --config-file /Users/jason/Library/Arduino15/arduino-cli.yaml config dump --format json�
12:13:07.809805 �[INIT --- : Arduino Data Dir -> 
12:13:07.809908 �textDocument/didOpen: �locked (waiting clangd)
12:13:07.809929   textDocument/didOpen: clangd startup failed: quitting Language server

Because Go doesn't check for exact matches between target structs and source data, the unmarshall call does not return an error in this situation.

		type cmdRes struct {
			Config struct {
				Directories struct {
					Data string `json:"data"`
				} `json:"directories"`
			} `json:"config"` // <--- THIS WRAPPER OBJECT DOESN'T EXIST IN THE JSON
		}
		var res cmdRes
		if err := json.Unmarshal(cmdOutput.Bytes(), &res); err != nil { // GO DOESN'T CARE THOUGH - NO ERROR!
			return nil, errors.Errorf("parsing arduino-cli output: %s", err)
		}

We end up with a zero-valued struct that happily returns an empty string as the data directory.

When we later turn this into a path with dataDirPath := paths.New(dataDir), we end up with a nil pointer, as the paths constructor returns nil on empty string input.

file: [email protected]/paths.go

func New(path ...string) *Path {
	if len(path) == 0 {
		return nil
	}
    // ...

The current version of the code fails to check for this condition.

Finally, when we pass this nil pointer along to clangd, it chokes, resulting in the vague error message about clangd startup failing.

		// Retrieve data folder
		dataFolder, err := ls.extractDataFolderFromArduinoCLI(logger) // <- EQUAL TO NIL
		if err != nil {
			logger.Logf("error retrieving data folder from arduino-cli: %s", err) // <- REMEMBER, NO ERROR GENERATED!
			return
		}

		// Start clangd
		ls.Clangd = newClangdLSPClient(logger, dataFolder, ls) // <- BOOM.

The fix in this PR updates the code to work with the current output, but it is not backward compatible!

I'll let someone else decide what to do next, but I wanted to make this patch (and explanation — pardon its length) available for anyone else running into this issue.

Environment Detail

• Arch: AARM64
• OS: MacOS Sonoma 14.3.1
• IDE: neovim
  neovim LSP config for arduino-language-server using nvim-lspconfig

    require('lspconfig').arduino_language_server.setup({
      on_attach = on_attach,
      capabilities = capabilities,
      cmd = {
        'arduino-language-server',
        '-cli-config',
        '/Users/jason/Library/Arduino15/arduino-cli.yaml',
        -- '-log' -- Warning: creates large log files in your project's root directory!
      },
    })

Additional Information for Passerbys

If you encounter this issue but are unfamiliar with Go, here's how to patch it quickly while the maintainers decide what to do with this PR.

Assuming you originally installed arduino-language-server with go install github.com/arduino/arduino-language-server, you can replace it with a locally patched version via:

# Clone this repository to your local computer (requires the git CLI)
git clone [email protected]:arduino/arduino-language-server
cd arduino-language-server
# Pull down this PR into a local branch named `patched`
git fetch origin pull/182/head:patched
# Checkout the new branch
git checkout patched
# Build and install this version -- replacing your existing copy
go install

[CLAassistant]

All committers have signed the CLA.

[alessio-perugini]

@jmcclell Your detailed description is much appreciated.
I don't know which version of the Arduino LSP you're running, but I suspect you're using the one on the tip of the main branch.
If you're using the arduino-cli v0.35.3, ensure you're running the arduino language server 0.7.6.

The arduino-cli (>= v0.36.0-rc.1) is currently undergoing a significant amount of breaking changes while moving forward to v1.0.0 which will be released in the upcoming months. To allow the IDE team to test such breaking changes, we are updating the LS to be compatible with the latest changes of the cli. For this reason, the latest commits present in the master branch of this repository will not work with the previous version of the cli (< v0.36.0-rc.1).

The panic you're experiencing is somewhat expected because it's interacting with a cli version that doesn't respect the new API. Pinning the LS to 0.7.6 should solve your problem.

You mentioned that you're using neovim, if you're managing the LSPs with mason you could change your lua config by pinning the LS version when passing the name to the ensure_installed list [email protected]. By doing so when you update the arduinoLS version on mason it should still load the pinned version until you manually uninstall it.

[jmcclell]

Ah! That explains it :) I haven't done any Arduino development in quite some time so I was starting fresh with my IDE setup and just assumed the LSP was behind the latest release of the CLI. 🤦🏻 Will pin the version. 🙇🏻

I will mention that I installed the LSP via go install with @latest, which is probably what most newcomers (who aren't using the official IDE) are going to do, I would think. Unless I'm the odd man out, having not been in this ecosystem for a while? You might consider putting a notice prominently in the README while these breaking changes are underway. 🤷🏻

[alessio-perugini]

@jmcclell Yep go install github.com/arduino/arduino-language-server@latest takes the latest commit on the master branch.

You might consider putting a notice prominently in the README

Usually what is sitting in the master/main/latest branch is considered unstable, but adding a notice won't harm anyone. 🤓 Tracking at #183

I appreciate your patience 💪