lexical-lsp / lexical

Lexical is a next-generation elixir language server
776 stars 77 forks source link

Always treat ranges as exclusive in `Range.contains?/2` #763

Closed zachallaun closed 4 weeks ago

zachallaun commented 4 weeks ago

Range.contains?/2 was treating the end-position of ranges as inclusive for single-line ranges, but exclusive for multi-line ranges. Our ranges should always be exclusive of the end character, however, so this was a bug. There were two other bugs/compensations that were counteracting it (at least in some cases), however:

  1. Cursor positions in tests

There were a lot of tests with cursor positions like this:

foo| = :foo

However, this cursor is actually "pointing" to the space after foo, not to the last character of foo:

foo = :foo
#  ^

Fixing Range.contains?/2 caused all of the tests with those kinds of cursor positions to break, but I believe those tests were incorrect in the first place. I changed those cursor positions to point to the first character of the entity instead (|foo).

  1. String ranges with a negative end offset

When detecting in strings, a number of fetch_range calls used a -1 end offset. This counteracted the inclusiveness of the end character in Range.contains?, but once that was fixed, string ranges became incorrect (short one character).

zachallaun commented 4 weeks ago

Closing in favor of #768