check the gopls version and update if outdated #2719
Conversation
|
@stamblerre If the main change is only in the Your older PR #2701 has been merged and I have updated PR #2703 with the latest changes from the master branch along with a few other fixes |
|
This change adds a few dependencies, so I figured it would make sense to logically separate it from the previous one. I'm happy to wait for #2703 to be merged, and then I can resolve the merge conflicts so that you can review this one. Thanks! |
|
@stamblerre Your other PRs have been merged to master. Once you resolve the merge conflicts in this PR, I'll start reviewing. |
|
Thanks for the reviews! I just resolved the merge conflicts here, so this change is ready for review. |
src/goInstallTools.ts
Outdated
| @@ -112,7 +112,7 @@ export function installTools(missing: Tool[], goVersion: SemVersion): Promise<vo | |||
| // This ensures that users get the latest tagged version, rather than master, | |||
| // which may be unstable. | |||
| let modulesOff = false; | |||
| if (isBelow(goVersion, 1, 11)) { | |||
| if (semver.lt(goVersion, '1.11.0')) { | |||
There was a problem hiding this comment.
How does semver.lt behave when the first argument is null?
Same question for gt or other comparison functions that are being used.
In our case goVersion will be null when using Go from the tip
You will need to add a null check on goVersion every time we use the lt, gt function
There was a problem hiding this comment.
I reworked some of the Go version handling to tackle developer versions. There's definitely more that can be done to improve handling, but this should work for now.
| // | ||
| // golang.org/x/tools/gopls@v0.1.3 | ||
| // | ||
| const split = moduleVersion.trim().split('@'); |
There was a problem hiding this comment.
moduleVersion can be undefined if lines[1] had no ' '
There was a problem hiding this comment.
I don't believe it would ever be. I tested a few cases, and in the case where there is no element to split on, split just returns an array of 1 element containing the original string.
src/goPackages.ts
Outdated
| @@ -6,8 +6,9 @@ | |||
| import vscode = require('vscode'); | |||
| import cp = require('child_process'); | |||
| import path = require('path'); | |||
| import semver = require('semver'); | |||
There was a problem hiding this comment.
This import is no longer used.
| import semver = require('semver'); |
src/goTools.ts
Outdated
| @@ -5,8 +5,9 @@ | |||
|
|
|||
| 'use strict'; | |||
|
|
|||
| import { SemVersion, isAbove, getGoConfig, isBelow } from './util'; | |||
| import vscode = require('vscode'); | |||
There was a problem hiding this comment.
This import is not needed.
| import vscode = require('vscode'); |
src/goVet.ts
Outdated
| @@ -4,8 +4,9 @@ | |||
| *--------------------------------------------------------*/ | |||
|
|
|||
| import path = require('path'); | |||
| import semver = require('semver'); | |||
There was a problem hiding this comment.
Import no longer needed.
| import semver = require('semver'); |
src/testUtils.ts
Outdated
| @@ -6,6 +6,7 @@ import cp = require('child_process'); | |||
| import path = require('path'); | |||
| import util = require('util'); | |||
| import vscode = require('vscode'); | |||
| import semver = require('semver'); | |||
There was a problem hiding this comment.
Import no longer needed.
| import semver = require('semver'); |
|
Fixed all the unused imports, sorry about that. I wonder why the linter didn't report them. |
Sorry about the huge diff, this PR is actually stacked on top of all of my previous ones. The main interesting change is at the very end of the
goLanguageServer.tsfile, where we detect thegoplsversion and determine if it is the most recent one.