🐛 Fix IndexMap's toInnerRange and toOuterRange returning extend…

…ed ranges (#1124)

packages/core/src/source/IndexMap.ts

@@ -11,13 +11,12 @@ export namespace IndexMap {
 		offset: number,
 		from: 'inner' | 'outer',
 		to: 'inner' | 'outer',
-		isEndOffset: boolean,
 	): number {
 		let ans = offset
 
 		for (const pair of map) {
-			if (Range.contains(pair[from], offset, isEndOffset)) {
-				return isEndOffset ? pair[to].end : pair[to].start
+			if (Range.contains(pair[from], offset)) {
+				return pair[to].start
 			} else if (Range.endsBefore(pair[from], offset)) {
 				ans = offset - pair[from].end + pair[to].end
 			} else {
@@ -29,24 +28,24 @@ export namespace IndexMap {
 	}
 
 	export function toInnerOffset(map: IndexMap, offset: number): number {
-		return convertOffset(map, offset, 'outer', 'inner', false)
+		return convertOffset(map, offset, 'outer', 'inner')
 	}
 
 	export function toInnerRange(map: IndexMap, outer: Range): Range {
 		return Range.create(
 			toInnerOffset(map, outer.start),
-			convertOffset(map, outer.end, 'outer', 'inner', true),
+			toInnerOffset(map, outer.end),
 		)
 	}
 
 	export function toOuterOffset(map: IndexMap, offset: number): number {
-		return convertOffset(map, offset, 'inner', 'outer', false)
+		return convertOffset(map, offset, 'inner', 'outer')
 	}
 
 	export function toOuterRange(map: IndexMap, inner: Range): Range {
 		return Range.create(
 			toOuterOffset(map, inner.start),
-			convertOffset(map, inner.end, 'inner', 'outer', true),
+			toOuterOffset(map, inner.end),
 		)
 	}
 

packages/core/src/source/Source.ts

@@ -110,10 +110,10 @@ export class ReadonlySource {
 			cursor++
 		) {
 			const c = this.string.charAt(cursor)
-			if (c === CR || c === LF) {
+			if (Source.isNewline(c)) {
 				break
 			}
-			if (!(c === ' ' || c === '\t')) {
+			if (!Source.isSpace(c)) {
 				return true
 			}
 		}
@@ -176,7 +176,7 @@ export class Source extends ReadonlySource {
 
 	/**
 	 * Skips the current character.
-	 * @param step The step to skip. @default 1
+	 * @param step The step to skip. Defaults to 1
 	 */
 	skip(step = 1): this {
 		this.innerCursor += step
@@ -313,24 +313,26 @@ export class Source extends ReadonlySource {
 		this.readRemaining()
 		return this
 	}
+}
 
-	static isDigit(c: string): c is Digit {
+export namespace Source {
+	export function isDigit(c: string): c is Digit {
 		return c >= '0' && c <= '9'
 	}
 
-	static isBrigadierQuote(c: string): c is '"' | "'" {
+	export function isBrigadierQuote(c: string): c is '"' | "'" {
 		return c === '"' || c === "'"
 	}
 
-	static isNewline(c: string): c is Newline {
+	export function isNewline(c: string): c is Newline {
 		return c === '\r\n' || c === '\r' || c === '\n'
 	}
 
-	static isSpace(c: string): c is Space {
+	export function isSpace(c: string): c is Space {
 		return c === ' ' || c === '\t'
 	}
 
-	static isWhitespace(c: string): c is Whitespace {
+	export function isWhitespace(c: string): c is Whitespace {
 		return Source.isSpace(c) || Source.isNewline(c)
 	}
 }

packages/core/test/source/IndexMap.spec.ts

@@ -4,65 +4,67 @@ import snapshot from 'snap-shot-it'
 import { IndexMap, Range } from '../../lib/index.js'
 
 describe('IndexMap', () => {
-	/*
-	 * Index Tens - 0000000000111111111122222222223
-	 * Index Ones - 0123456789012345678901234567890
-	 * Outer      -             "foo\"bar\u00a7qux"
-	 * Inner      - foo"bar§qux
-	 */
-	const map: IndexMap = [
-		{ outer: Range.create(13, 13), inner: Range.create(0, 0) },
-		{ outer: Range.create(16, 18), inner: Range.create(3, 4) },
-		{ outer: Range.create(21, 27), inner: Range.create(7, 8) },
-	]
-	const toInnerCases: { input: number; expected: number }[] = [
-		{ input: 13, expected: 0 },
-		{ input: 14, expected: 1 },
-		{ input: 15, expected: 2 },
-		{ input: 16, expected: 3 },
-		{ input: 17, expected: 3 },
-		{ input: 18, expected: 4 },
-		{ input: 19, expected: 5 },
-		{ input: 20, expected: 6 },
-		{ input: 21, expected: 7 },
-		{ input: 22, expected: 7 },
-		{ input: 23, expected: 7 },
-		{ input: 24, expected: 7 },
-		{ input: 25, expected: 7 },
-		{ input: 26, expected: 7 },
-		{ input: 27, expected: 8 },
-		{ input: 28, expected: 9 },
-		{ input: 29, expected: 10 },
-		{ input: 30, expected: 11 },
-	]
-	const toOuterCases: { input: number; expected: number }[] = [
-		{ input: 0, expected: 13 },
-		{ input: 1, expected: 14 },
-		{ input: 2, expected: 15 },
-		{ input: 3, expected: 16 },
-		{ input: 4, expected: 18 },
-		{ input: 5, expected: 19 },
-		{ input: 6, expected: 20 },
-		{ input: 7, expected: 21 },
-		{ input: 8, expected: 27 },
-		{ input: 9, expected: 28 },
-		{ input: 10, expected: 29 },
-		{ input: 11, expected: 30 },
-	]
-	for (const method of ['toInnerOffset', 'toOuterOffset'] as const) {
-		describe(`${method}()`, () => {
-			for (
-				const { input, expected } of method === 'toInnerOffset'
-					? toInnerCases
-					: toOuterCases
-			) {
-				it(`Should return ${expected} for ${input}`, () => {
-					const actual = IndexMap[method](map, input)
-					assert.strictEqual(actual, expected)
-				})
-			}
-		})
-	}
+	describe('to<Inner/Outer>Offset', () => {
+		/*
+		 * Index Tens - 0000000000111111111122222222223
+		 * Index Ones - 0123456789012345678901234567890
+		 * Outer      -             "foo\"bar\u00a7qux"
+		 * Inner      - foo"bar§qux
+		 */
+		const map: IndexMap = [
+			{ outer: Range.create(13, 13), inner: Range.create(0, 0) },
+			{ outer: Range.create(16, 18), inner: Range.create(3, 4) },
+			{ outer: Range.create(21, 27), inner: Range.create(7, 8) },
+		]
+		const toInnerCases: { input: number; expected: number }[] = [
+			{ input: 13, expected: 0 },
+			{ input: 14, expected: 1 },
+			{ input: 15, expected: 2 },
+			{ input: 16, expected: 3 },
+			{ input: 17, expected: 3 },
+			{ input: 18, expected: 4 },
+			{ input: 19, expected: 5 },
+			{ input: 20, expected: 6 },
+			{ input: 21, expected: 7 },
+			{ input: 22, expected: 7 },
+			{ input: 23, expected: 7 },
+			{ input: 24, expected: 7 },
+			{ input: 25, expected: 7 },
+			{ input: 26, expected: 7 },
+			{ input: 27, expected: 8 },
+			{ input: 28, expected: 9 },
+			{ input: 29, expected: 10 },
+			{ input: 30, expected: 11 },
+		]
+		const toOuterCases: { input: number; expected: number }[] = [
+			{ input: 0, expected: 13 },
+			{ input: 1, expected: 14 },
+			{ input: 2, expected: 15 },
+			{ input: 3, expected: 16 },
+			{ input: 4, expected: 18 },
+			{ input: 5, expected: 19 },
+			{ input: 6, expected: 20 },
+			{ input: 7, expected: 21 },
+			{ input: 8, expected: 27 },
+			{ input: 9, expected: 28 },
+			{ input: 10, expected: 29 },
+			{ input: 11, expected: 30 },
+		]
+		for (const method of ['toInnerOffset', 'toOuterOffset'] as const) {
+			describe(`${method}()`, () => {
+				for (
+					const { input, expected } of method === 'toInnerOffset'
+						? toInnerCases
+						: toOuterCases
+				) {
+					it(`Should return ${expected} for ${input}`, () => {
+						const actual = IndexMap[method](map, input)
+						assert.strictEqual(actual, expected)
+					})
+				}
+			})
+		}
+	})
 
 	describe('merge()', () => {
 		it('Should merge correctly', () => {
@@ -110,4 +112,102 @@ describe('IndexMap', () => {
 			snapshot(mergedMap)
 		})
 	})
+
+	describe('to<Inner/Outer>Range', () => {
+		describe('foo"bar§qux <=> foo\\"bar\\u00a7qux', () => {
+			/*
+			 * Index Tens - 000000000011111111112
+			 * Index Ones - 012345678901234567890
+			 * Outer      - foo\"bar\u00a7qux
+			 * Inner      - foo"bar§qux
+			 */
+			const indexMap = [
+				{ inner: Range.create(3, 4), outer: Range.create(3, 5) },
+				{ inner: Range.create(7, 8), outer: Range.create(8, 14) },
+			]
+			const toInnerCases = [
+				{
+					input: Range.create(0, 1),
+					expected: Range.create(0, 1),
+					name: '`f` -> `f`',
+				},
+				{
+					input: Range.create(3, 5),
+					expected: Range.create(3, 4),
+					name: '`\\"` -> `"`',
+				},
+				{ // (shifted left)
+					input: Range.create(7, 8),
+					expected: Range.create(6, 7),
+					name: '`r` -> `r`',
+				},
+				{
+					input: Range.create(8, 14),
+					expected: Range.create(7, 8),
+					name: '`\\u00a7` -> `§`',
+				},
+				{
+					input: Range.create(7, 14),
+					expected: Range.create(6, 8),
+					name: '`r\\u00a7` -> `r§`',
+				},
+				{
+					input: Range.create(7, 12),
+					expected: Range.create(6, 7),
+					name: '`r\\u00` -> `r`',
+				},
+				{
+					input: Range.create(7, 15),
+					expected: Range.create(6, 9),
+					name: '`r\\u00a7q` -> `r§q`',
+				},
+			]
+			const toOuterCases = [
+				{
+					input: Range.create(0, 1),
+					expected: Range.create(0, 1),
+					name: '`f` -> `f`',
+				},
+				{
+					input: Range.create(3, 4),
+					expected: Range.create(3, 5),
+					name: '`"` -> `"`',
+				},
+				{ // (shifted right)
+					input: Range.create(6, 7),
+					expected: Range.create(7, 8),
+					name: '`r` -> `r`',
+				},
+				{
+					input: Range.create(7, 8),
+					expected: Range.create(8, 14),
+					name: '`§` -> `\\u00a7`',
+				},
+				{
+					input: Range.create(6, 8),
+					expected: Range.create(7, 14),
+					name: '`r§` -> `r\\u00a7`',
+				},
+				{
+					input: Range.create(6, 9),
+					expected: Range.create(7, 15),
+					name: '`r§q` -> `r\\u00a7`',
+				},
+			]
+			for (const method of ['toInnerRange', 'toOuterRange'] as const) {
+				describe(`${method}()`, () => {
+					for (
+						const { input, expected, name } of method === 'toInnerRange'
+							? toInnerCases
+							: toOuterCases
+					) {
+						it(`Should convert ${Range.toString(input)} to ${Range.toString(expected)} (${name})`, () => {
+							const actual = IndexMap[method](indexMap, input)
+							assert.deepEqual(actual, expected)
+						})
+					}
+				})
+			}
+		})
+	})
 })

packages/core/test/source/Range.spec.ts

@@ -120,16 +120,30 @@ describe('Range', () => {
 					{ offset: 3, expected: false },
 				],
 			},
+			{
+				endInclusive: true,
+				range: Range.create(1, 2),
+				cases: [
+					{ offset: 0, expected: false },
+					{ offset: 1, expected: true },
+					{ offset: 2, expected: true },
+					{ offset: 3, expected: false },
+				],
+			},
 			{
 				range: Range.Full,
 				cases: [{ offset: 4, expected: true }],
 			},
-		] as const
-		for (const { range, cases } of suites) {
-			describe(`range ${Range.toString(range)}`, () => {
+		] as {
+			range: Range
+			cases: { offset: number; expected: boolean }[]
+			endInclusive?: boolean
+		}[]
+		for (const { range, cases, endInclusive = false } of suites) {
+			describe(`range ${Range.toString(range)}${endInclusive ? ' (endInclusive = true)' : ''}`, () => {
 				for (const { offset, expected } of cases) {
 					it(`Should return ${expected} for ${offset}`, () => {
-						const actual = Range.contains(range, offset)
+						const actual = Range.contains(range, offset, endInclusive)
 						assert.strictEqual(actual, expected)
 					})
 				}