Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion tools/lint-go-gopls.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,29 @@ IGNORE_PATTERNS=(
"is deprecated" # TODO: fix these
)

# Install gopls if not already installed, then use the installed binary for
# faster execution. Using 'go run' each time adds overhead.
"$GO" install "$GOPLS_PACKAGE"
GOPLS_BIN=$("$GO" env GOPATH)/bin/gopls
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 GOPLS_BIN path doesn't account for GOBIN environment variable

The script determines the gopls binary path using $("$GO" env GOPATH)/bin/gopls, but when the GOBIN environment variable is set, go install places binaries in $GOBIN instead of $GOPATH/bin.

When a user has GOBIN configured (a common setup for Go developers who want binaries in a custom location), the script will:

  1. Successfully install gopls (to $GOBIN/gopls)
  2. Look for gopls at $GOPATH/bin/gopls (wrong location)
  3. Fail the -x check and exit with "Error: Failed to install gopls"

This causes the lint step to fail with a misleading error message even though gopls was installed correctly. The proper approach would be to use $("$GO" env GOBIN) if set, falling back to $("$GO" env GOPATH)/bin otherwise.

Recommendation: Use the correct binary path that accounts for GOBIN:

GOPLS_BIN=$({ "$GO" env GOBIN; } | grep . || echo "$("$GO" env GOPATH)/bin")/gopls

Or more readably:

GOBIN_DIR=$("$GO" env GOBIN)
if [[ -z "$GOBIN_DIR" ]]; then
  GOBIN_DIR=$("$GO" env GOPATH)/bin
fi
GOPLS_BIN="$GOBIN_DIR/gopls"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


# Verify gopls was installed successfully
if [[ ! -x "$GOPLS_BIN" ]]; then
echo "Error: Failed to install gopls" >&2
exit 1
fi

# Parallelism is configurable via GOPLS_PARALLEL env var.
# Default to 1 (sequential) to avoid memory pressure on local machines.
# CI can set a higher value if desired (e.g., GOPLS_PARALLEL=4).
PARALLEL=${GOPLS_PARALLEL:-1}

# lint all go files with 'gopls check' and look for lines starting with the
# current absolute path, indicating a error was found. This is necessary
# because the tool does not set non-zero exit code when errors are found.
# ref: https://github.com/golang/go/issues/67078
ERROR_LINES=$("$GO" run "$GOPLS_PACKAGE" check -severity=warning "$@" 2>/dev/null | grep -E "^$PWD" | grep -vFf <(printf '%s\n' "${IGNORE_PATTERNS[@]}"));
# Use xargs with configurable parallelism (-P) to process files.
# Use null-delimited format (-0) to handle filenames with spaces correctly.
ERROR_LINES=$(printf '%s\0' "$@" | xargs -0 -P "$PARALLEL" -n 100 "$GOPLS_BIN" check -severity=warning 2>/dev/null | grep -E "^$PWD" | grep -vFf <(printf '%s\n' "${IGNORE_PATTERNS[@]}"));
NUM_ERRORS=$(echo -n "$ERROR_LINES" | wc -l)

if [ "$NUM_ERRORS" -eq "0" ]; then
Expand Down