Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

GRAPH-646: scip syntax java package namespace fix#63048

Merged
keynmol merged 8 commits into
mainfrom
GRAPH-646/scip-syntax-java-package-namespace-fix
Jun 5, 2024
Merged

GRAPH-646: scip syntax java package namespace fix#63048
keynmol merged 8 commits into
mainfrom
GRAPH-646/scip-syntax-java-package-namespace-fix

Conversation

@keynmol

@keynmol keynmol commented Jun 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes GRAPH-646

The tree in reproducer is:

(program ; [0, 0] - [4, 0]
  (package_declaration ; [0, 0] - [1, 16]
    (marker_annotation ; [0, 0] - [0, 11]
      name: (identifier)) ; [0, 1] - [0, 11]
    (scoped_identifier ; [1, 8] - [1, 15]
      scope: (identifier) ; [1, 8] - [1, 11]
      name: (identifier))) ; [1, 12] - [1, 15]
  (class_declaration ; [3, 0] - [3, 12]
    name: (identifier) ; [3, 6] - [3, 9]
    body: (class_body))) ; [3, 10] - [3, 12]

I think this was an error introduced in https://github.com/sourcegraph/sourcegraph/pull/53305/files#diff-0403a6bac5a125ef13ce5966a6c668eb3d5ca9c7fbb382d098f178c5d2c59f66R2-R6 - annotation implicitly being marked as opening of a scope?

Test plan

  • Reproducer + snapshot test

@cla-bot cla-bot Bot added the cla-signed label Jun 3, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jun 3, 2024
@keynmol keynmol force-pushed the GRAPH-646/scip-syntax-java-package-namespace-fix branch from 0ef292c to c1fb313 Compare June 3, 2024 17:09
@keynmol keynmol changed the title Graph 646/scip syntax java package namespace fix GRAPH-646: scip syntax java package namespace fix Jun 3, 2024
@kritzcreek kritzcreek self-requested a review June 3, 2024 17:14
@keynmol keynmol marked this pull request as ready for review June 4, 2024 12:11
kritzcreek
kritzcreek suggested changes Jun 4, 2024
View reviewed changes
Comment thread ...tes/syntax-analysis/src/snapshots/syntax_analysis__test__scip_snapshot_globals.java.snap.new Outdated

@kritzcreek kritzcreek Jun 4, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think .snap.new files are for files you still need to review with insta. We don't want to commit these and instead accept their changes into the .snap file.

@varungandhi-src varungandhi-src Jun 4, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's add .snap.new to .gitignore so that they don't accidentally get committed.

keynmol
keynmol commented Jun 5, 2024
View reviewed changes
Comment thread docker-images/syntax-highlighter/crates/scip-syntax/testdata/java/index.scip
@keynmol keynmol requested a review from kritzcreek June 5, 2024 09:34
kritzcreek
kritzcreek approved these changes Jun 5, 2024
View reviewed changes

@kritzcreek kritzcreek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@keynmol keynmol merged commit 507668f into main Jun 5, 2024
@keynmol keynmol deleted the GRAPH-646/scip-syntax-java-package-namespace-fix branch June 5, 2024 09:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@varungandhi-src varungandhi-src varungandhi-src left review comments

@kritzcreek kritzcreek kritzcreek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@keynmol @kritzcreek @varungandhi-src