Merge pull request #1672 from adalinesimonian/fix-ranges

fix: use correct positions for ranges

lib/node.d.ts

@@ -56,11 +56,11 @@ export interface Source {
    */
   input: Input
   /**
-   * The starting position of the node’s source.
+   * The inclusive starting position of the node’s source.
    */
   start?: Position
   /**
-   * The ending position of the node's source.
+   * The inclusive ending position of the node's source.
    */
   end?: Position
 }

lib/node.js

@@ -56,18 +56,13 @@ class Node {
 
   error(message, opts = {}) {
     if (this.source) {
-      if (opts.word || opts.endIndex) {
-        let { start, end } = this.rangeBy(opts)
-        return this.source.input.error(
-          message,
-          { line: start.line, column: start.column },
-          { line: end.line, column: end.column },
-          opts
-        )
-      }
-
-      let start = this.positionBy(opts)
-      return this.source.input.error(message, start.line, start.column, opts)
+      let { start, end } = this.rangeBy(opts)
+      return this.source.input.error(
+        message,
+        { line: start.line, column: start.column },
+        { line: end.line, column: end.column },
+        opts
+      )
     }
     return new CssSyntaxError(message)
   }
@@ -263,24 +258,55 @@ class Node {
   }
 
   rangeBy(opts) {
+    let start = {
+      line: this.source.start.line,
+      column: this.source.start.column
+    }
+    let end = this.source.end
+      ? {
+          line: this.source.end.line,
+          column: this.source.end.column + 1
+        }
+      : {
+          line: start.line,
+          column: start.column + 1
+        }
+
     if (opts.word) {
       let index = this.toString().indexOf(opts.word)
       if (index !== -1) {
-        return {
-          start: this.positionInside(index),
-          end: this.positionInside(index + opts.word.length)
+        start = this.positionInside(index)
+        end = this.positionInside(index + opts.word.length)
+      }
+    } else {
+      if (opts.start) {
+        start = {
+          line: opts.start.line,
+          column: opts.start.column
         }
+      } else if (opts.index) {
+        start = this.positionInside(opts.index)
       }
-    }
 
-    let { start } = this.source
-    let end = start
-    if (opts.index) {
-      start = this.positionInside(opts.index)
+      if (opts.end) {
+        end = {
+          line: opts.end.line,
+          column: opts.end.column
+        }
+      } else if (opts.endIndex) {
+        end = this.positionInside(opts.endIndex)
+      } else if (opts.index) {
+        end = this.positionInside(opts.index + 1)
+      }
     }
-    if (opts.endIndex) {
-      end = this.positionInside(opts.endIndex)
+
+    if (
+      end.line < start.line ||
+      (end.line === start.line && end.column <= start.column)
+    ) {
+      end = { line: start.line, column: start.column + 1 }
     }
+
     return { start, end }
   }
 

lib/warning.d.ts

@@ -1,3 +1,4 @@
+import { RangePosition } from './css-syntax-error.js'
 import Node from './node.js'
 
 export interface WarningOptions {
@@ -21,6 +22,16 @@ export interface WarningOptions {
    */
   endIndex?: number
 
+  /**
+   * Start position, inclusive, in CSS node string that caused the warning.
+   */
+  start?: RangePosition
+
+  /**
+   * End position, exclusive, in CSS node string that caused the warning.
+   */
+  end?: RangePosition
+
   /**
    * Name of the plugin that created this warning. `Result#warn` fills
    * this property automatically.

lib/warning.js

@@ -6,27 +6,11 @@ class Warning {
     this.text = text
 
     if (opts.node && opts.node.source) {
-      if (
-        opts.word ||
-        typeof opts.endLine === 'number' ||
-        typeof opts.endIndex === 'number'
-      ) {
-        let range = opts.node.rangeBy(opts)
-        this.line = range.start.line
-        this.column = range.start.column
-
-        if (
-          range.start.line !== range.end.line ||
-          range.start.column !== range.end.column
-        ) {
-          this.endLine = range.end.line
-          this.endColumn = range.end.column
-        }
-      } else {
-        let pos = opts.node.positionBy(opts)
-        this.line = pos.line
-        this.column = pos.column
-      }
+      let range = opts.node.rangeBy(opts)
+      this.line = range.start.line
+      this.column = range.start.column
+      this.endLine = range.end.line
+      this.endColumn = range.end.column
     }
 
     for (let opt in opts) this[opt] = opts[opt]

test/warning.test.ts

@@ -62,13 +62,27 @@ test('has line and column is undefined by default', () => {
   let warning = new Warning('text')
   type(warning.line, 'undefined')
   type(warning.column, 'undefined')
+  type(warning.endLine, 'undefined')
+  type(warning.endColumn, 'undefined')
 })
 
-test('gets position from node', () => {
+test('gets range from node', () => {
   let root = parse('a{}')
   let warning = new Warning('text', { node: root.first })
   is(warning.line, 1)
   is(warning.column, 1)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 4)
+})
+
+test('gets range from node without end', () => {
+  let root = parse('a{}')
+  root.first!.source!.end = undefined
+  let warning = new Warning('text', { node: root.first })
+  is(warning.line, 1)
+  is(warning.column, 1)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 2)
 })
 
 test('gets range from word', () => {
@@ -80,11 +94,13 @@ test('gets range from word', () => {
   is(warning.endColumn, 4)
 })
 
-test('gets position from index', () => {
+test('gets range from index', () => {
   let root = parse('a b{}')
   let warning = new Warning('text', { node: root.first, index: 2 })
   is(warning.line, 1)
   is(warning.column, 3)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 4)
 })
 
 test('gets range from index and endIndex', () => {
@@ -96,4 +112,59 @@ test('gets range from index and endIndex', () => {
   is(warning.endColumn, 4)
 })
 
+test('gets range from start', () => {
+  let root = parse('a b{}')
+  let warning = new Warning('text', {
+    node: root.first,
+    start: { line: 1, column: 3 }
+  })
+  is(warning.line, 1)
+  is(warning.column, 3)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 6)
+})
+
+test('gets range from end', () => {
+  let root = parse('a b{}')
+  let warning = new Warning('text', {
+    node: root.first,
+    end: { line: 1, column: 3 }
+  })
+  is(warning.line, 1)
+  is(warning.column, 1)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 3)
+})
+
+test('gets range from start and end', () => {
+  let root = parse('a b{}')
+  let warning = new Warning('text', {
+    node: root.first,
+    start: { line: 1, column: 3 },
+    end: { line: 1, column: 4 }
+  })
+  is(warning.line, 1)
+  is(warning.column, 3)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 4)
+})
+
+test('always returns exclusive ends', () => {
+  let root = parse('a b{}')
+  let warning = new Warning('text', { node: root.first, index: 1, endIndex: 1 })
+  is(warning.line, 1)
+  is(warning.column, 2)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 3)
+})
+
+test('always returns valid ranges', () => {
+  let root = parse('a b{}')
+  let warning = new Warning('text', { node: root.first, index: 2, endIndex: 1 })
+  is(warning.line, 1)
+  is(warning.column, 3)
+  is(warning.endLine, 1)
+  is(warning.endColumn, 4)
+})
+
 test.run()