fix: not use negative indices for remove in empty string (#281)

Co-authored-by: Anthony Fu <[email protected]>
Rich-Harris · Jul 29, 2024 · 5c1cba0 · 5c1cba0
1 parent d6b2171
commit 5c1cba0
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 12 deletions.
diff --git a/benchmark/data.js b/benchmark/data.js
@@ -663,10 +663,12 @@ class MagicString {
     return this.intro + lineStr;
   }
   slice(start = 0, end = this.original.length) {
-    while (start < 0)
-      start += this.original.length;
-    while (end < 0)
-      end += this.original.length;
+    if (this.original.length !== 0) {
+      while (start < 0)
+        start += this.original.length;
+      while (end < 0)
+        end += this.original.length;
+    }
     let result = "";
     let chunk = this.firstChunk;
     while (chunk && (chunk.start > start || chunk.end <= start)) {

diff --git a/src/MagicString.js b/src/MagicString.js
@@ -360,8 +360,10 @@ export default class MagicString {
 	update(start, end, content, options) {
 		if (typeof content !== 'string') throw new TypeError('replacement content must be a string');
 
-		while (start < 0) start += this.original.length;
-		while (end < 0) end += this.original.length;
+		if (this.original.length !== 0) {
+			while (start < 0) start += this.original.length;
+			while (end < 0) end += this.original.length;
+		}
 
 		if (end > this.original.length) throw new Error('end is out of bounds');
 		if (start === end)
@@ -469,8 +471,10 @@ export default class MagicString {
 	}
 
 	remove(start, end) {
-		while (start < 0) start += this.original.length;
-		while (end < 0) end += this.original.length;
+		if (this.original.length !== 0) {
+			while (start < 0) start += this.original.length;
+			while (end < 0) end += this.original.length;
+		}
 
 		if (start === end) return this;
 
@@ -497,8 +501,10 @@ export default class MagicString {
 	}
 
 	reset(start, end) {
-		while (start < 0) start += this.original.length;
-		while (end < 0) end += this.original.length;
+		if (this.original.length !== 0) {
+			while (start < 0) start += this.original.length;
+			while (end < 0) end += this.original.length;
+		}
 
 		if (start === end) return this;
 
@@ -564,8 +570,10 @@ export default class MagicString {
 	}
 
 	slice(start = 0, end = this.original.length) {
-		while (start < 0) start += this.original.length;
-		while (end < 0) end += this.original.length;
+		if (this.original.length !== 0) {
+			while (start < 0) start += this.original.length;
+			while (end < 0) end += this.original.length;
+		}
 
 		let result = '';
 

diff --git a/test/MagicString.js b/test/MagicString.js
@@ -1234,6 +1234,19 @@ describe('MagicString', () => {
 
 			assert.equal(s.toString(), 'abchidejkl');
 		});
+
+		it('should accept negative indices', () => {
+			const s = new MagicString('abcde');
+			// "abcde"
+			//     ^
+			s.remove(-2, -1);
+			assert.equal(s.toString(), 'abce');
+		});
+
+		it('should throw error when using negative indices with empty string', () => {
+			const s = new MagicString('');
+			assert.throws(() => s.remove(-2, -1), /Error: Character is out of bounds/);
+		});
 	});
 
 	describe('reset', () => {