Skip to content

Commit

Permalink
fix: Validate local symbols properly
Browse files Browse the repository at this point in the history
  • Loading branch information
varungandhi-src committed Oct 23, 2023
1 parent 1045a4d commit 2fa9f60
Show file tree
Hide file tree
Showing 9 changed files with 1,916 additions and 1,825 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- uses: ./.github/actions/asdf
with:
rust: true
- run: cargo check
- run: cargo test
working-directory: bindings/rust
- run: cargo check
working-directory: cmd/scip/tests/reprolang
3 changes: 3 additions & 0 deletions bindings/go/scip/scip.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 38 additions & 5 deletions bindings/go/scip/symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,41 @@ import (
"github.com/sourcegraph/sourcegraph/lib/errors"
)

// IsGlobalSymbol returns true if the symbol is obviously not a local symbol.
//
// CAUTION: Does not perform full validation of the symbol string's contents.
func IsGlobalSymbol(symbol string) bool {
return !IsLocalSymbol(symbol)
return !strings.HasPrefix(symbol, "local ")
}

func isSimpleIdentifier(s string) bool {
for _, c := range s {
if ('0' <= c && c <= '9') || ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') ||
c == '$' || c == '+' || c == '-' || c == '_' {
continue
}
return false
}
return true
}

func IsLocalSymbol(symbol string) bool {
return strings.HasPrefix(symbol, "local ")
if !strings.HasPrefix(symbol, "local ") {
return false
}
suffix := symbol[6:]
return len(suffix) > 0 && isSimpleIdentifier(suffix)
}

func tryParseLocalSymbol(symbol string) (string, error) {
if !strings.HasPrefix(symbol, "local ") {
return "", nil
}
suffix := symbol[6:]
if len(suffix) > 0 && isSimpleIdentifier(suffix) {
return suffix, nil
}
return "", errors.Newf("expected format 'local <simple-identifier>' but got: %v", symbol)
}

// ParseSymbol parses an SCIP string into the Symbol message.
Expand All @@ -24,14 +53,18 @@ func ParseSymbol(symbol string) (*Symbol, error) {
// ParsePartialSymbol parses an SCIP string into the Symbol message
// with the option to exclude the `.Descriptor` field.
func ParsePartialSymbol(symbol string, includeDescriptors bool) (*Symbol, error) {
local, err := tryParseLocalSymbol(symbol)
if err != nil {
return nil, err
}
if local != "" {
return newLocalSymbol(local), nil
}
s := newSymbolParser(symbol)
scheme, err := s.acceptSpaceEscapedIdentifier("scheme")
if err != nil {
return nil, err
}
if scheme == "local" {
return newLocalSymbol(string(s.Symbol[s.index:])), nil
}
manager, err := s.acceptSpaceEscapedIdentifier("package manager")
if err != nil {
return nil, err
Expand Down
6 changes: 5 additions & 1 deletion bindings/go/scip/symbol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,15 @@ func TestParseSymbol(t *testing.T) {
}
}

func TestParseSymbolNoCrash(t *testing.T) {
func TestParseSymbolError(t *testing.T) {
for _, symbolName := range []string{
"",
"lsif-java maven package 1.0.0",
"lsif-java maven package 1.0.0 java/io/File#Entry.trailingstring",
"lsif-java maven package 1.0.0 java/io/File#Entry.unrecognizedSuffix@",
"local 🧠",
"local ",
"local &&&",
} {

if _, err := ParseSymbol(symbolName); err == nil {
Expand Down
Loading

0 comments on commit 2fa9f60

Please sign in to comment.