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

Adds unit tests for syntactic usages#63900

Merged
kritzcreek merged 6 commits into
mainfrom
christoph/test-syntactic
Jul 22, 2024
Merged

Adds unit tests for syntactic usages#63900
kritzcreek merged 6 commits into
mainfrom
christoph/test-syntactic

Conversation

@kritzcreek

@kritzcreek kritzcreek commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Closes https://linear.app/sourcegraph/issue/GRAPH-735/test-syntactic-usages

Uses the new MappedIndex abstraction from #63781 to implement some proper unit tests for syntacticUsagesImpl.

Test plan

PR adds a bunch of tests

@cla-bot cla-bot Bot added the cla-signed label Jul 18, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jul 18, 2024
@kritzcreek kritzcreek changed the title WIP: unit tests for syntactic usages Adds unit tests for syntactic usages Jul 18, 2024
@kritzcreek kritzcreek marked this pull request as ready for review July 18, 2024 09:51
@kritzcreek kritzcreek requested a review from keynmol July 18, 2024 09:51
@kritzcreek kritzcreek force-pushed the christoph/test-syntactic branch 2 times, most recently from b31dbbd to 43dbc8d Compare July 18, 2024 09:53

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 bet it felt exhilirating to use global mutable state :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've read the ancient scrolls (and so has Claude apparently):

What are the oldest references to the gensym function

A\ The concept of "gensym" (short for "generate symbol") has been around in programming for quite a long time, particularly in the Lisp family of languages. Here are some of the oldest known references to gensym:

  1. Lisp 1.5 (1962): While not called "gensym" explicitly, the concept of generating unique symbols was present in the original Lisp 1.5 manual by John McCarthy. The function was called "GENSYM" in later Lisp implementations.
  2. MacLisp (late 1960s): MacLisp, developed at MIT's Project MAC, included a gensym function. It was one of the earliest widely-used implementations to feature gensym.
  3. Interlisp (early 1970s): Interlisp, developed at Xerox PARC, also included a gensym function.
  4. Common Lisp (1984): The Common Lisp language specification, which aimed to standardize various Lisp dialects, included gensym as a standard function.
  5. Scheme (1975): While not called "gensym", Scheme (a Lisp dialect) introduced the concept of hygiene in macros, which addresses similar concerns as gensym.
  6. PL/I (1964): While not called "gensym", PL/l had a built-in function UNSPEC which could be used to generate unique identifiers.
    The gensym function became particularly important in macro systems and symbolic computation, where generating unique symbols is crucial to avoid name conflicts. Its widespread use in Lisp dialects made it a well-known concept in the programming language community by the 1970s.
    It's worth noting that while these are some of the earliest references, the exact origin of the term "gensym" is somewhat obscure, and it may have been in use informally before appearing in official documentation or implementations.

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.

there's a require.NoErrorf which can do this

Comment on lines 51 to 66

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.

Can you explain/add a comment about what exactly is being etsted here? I don't think I understand by reading the assertions and the test name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's being tested here, is that when search returns two matches, and we have a syntactic occurrence overlapping with one of them, then search-based usages will only return the one non-overlapping match.
There's some confusing naming here, I should call the refRange something like comment or search range.

Definitely need to be clearer here, thanks!

@kritzcreek kritzcreek requested a review from keynmol July 22, 2024 03:02

@keynmol keynmol 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.

I was able to follow all the test cases 👍

@kritzcreek kritzcreek force-pushed the christoph/test-syntactic branch from e99e409 to 14a7e04 Compare July 22, 2024 12:21
@kritzcreek kritzcreek enabled auto-merge (squash) July 22, 2024 12:22
@kritzcreek kritzcreek merged commit 3ef0be1 into main Jul 22, 2024
@kritzcreek kritzcreek deleted the christoph/test-syntactic branch July 22, 2024 13:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants