Repair character positions without touching the configuration

JSHint reports physical character positions instead of a character
index, i.e. all tab characters are multiplied with the (configurable)
indent. We used to set the option "indent" to 1 to get the real
character index. But this setting could be overwritten in a custom
configuration.

Now we leave the configuration untouched and repair the character
position based on the configured or default indent.

Author: ralfstx

com.eclipsesource.jshint.test/src/com/eclipsesource/jshint/Configuration_Test.java

@@ -27,80 +27,75 @@ public void setUp() {
 
   @Test
   public void emptyAfterCreation() {
-    assertEquals( "{\"indent\": 1}", configuration.toJson() );
+    assertEquals( "{}", configuration.toJson() );
   }
 
   @Test
   public void addOneOption() {
     configuration.addOption( "foo", true );
 
-    assertEquals( "{\"indent\": 1, \"foo\": true}", configuration.toJson() );
+    assertEquals( "{\"foo\": true}", configuration.toJson() );
   }
 
   @Test
   public void addSeparateOptions() {
     configuration.addOption( "foo", true );
     configuration.addOption( "bar", false );
 
-    assertEquals( "{\"indent\": 1, \"foo\": true, \"bar\": false}",
-                  configuration.toJson() );
+    assertEquals( "{\"foo\": true, \"bar\": false}", configuration.toJson() );
   }
 
   @Test
   public void addSeparateOptionsWithChaining() {
     configuration.addOption( "foo", true ).addOption( "bar", false );
 
-    assertEquals( "{\"indent\": 1, \"foo\": true, \"bar\": false}",
-                  configuration.toJson() );
+    assertEquals( "{\"foo\": true, \"bar\": false}", configuration.toJson() );
   }
 
   @Test
   public void addSameOptionTwice() {
     configuration.addOption( "foo", true );
     configuration.addOption( "foo", false );
 
-    assertEquals( "{\"indent\": 1, \"foo\": false}", configuration.toJson() );
+    assertEquals( "{\"foo\": false}", configuration.toJson() );
   }
 
   @Test
   public void addSamePredefTwice() {
     configuration.addPredefined( "foo", true );
     configuration.addPredefined( "foo", false );
 
-    assertEquals( "{\"predef\": {\"foo\": false}, \"indent\": 1}", configuration.toJson() );
+    assertEquals( "{\"predef\": {\"foo\": false}}", configuration.toJson() );
   }
 
   @Test
   public void addOnePredef() {
     configuration.addPredefined( "foo", true );
 
-    assertEquals( "{\"predef\": {\"foo\": true}, \"indent\": 1}", configuration.toJson() );
+    assertEquals( "{\"predef\": {\"foo\": true}}", configuration.toJson() );
   }
 
   @Test
   public void addSeparatePredefsWithChaining() {
     configuration.addPredefined( "foo", true ).addPredefined( "bar", false );
 
-    assertEquals( "{\"predef\": {\"foo\": true, \"bar\": false}, \"indent\": 1}",
-                  configuration.toJson() );
+    assertEquals( "{\"predef\": {\"foo\": true, \"bar\": false}}", configuration.toJson() );
   }
 
   @Test
   public void addSeparatePredefs() {
     configuration.addPredefined( "foo", true );
     configuration.addPredefined( "bar", false );
 
-    assertEquals( "{\"predef\": {\"foo\": true, \"bar\": false}, \"indent\": 1}",
-                  configuration.toJson() );
+    assertEquals( "{\"predef\": {\"foo\": true, \"bar\": false}}", configuration.toJson() );
   }
 
   @Test
   public void addPredefAndOption() {
     configuration.addPredefined( "foo", true );
     configuration.addOption( "bar", false );
 
-    assertEquals( "{\"predef\": {\"foo\": true}, \"indent\": 1, \"bar\": false}",
-                  configuration.toJson() );
+    assertEquals( "{\"predef\": {\"foo\": true}, \"bar\": false}", configuration.toJson() );
   }
 
 }

com.eclipsesource.jshint.test/src/com/eclipsesource/jshint/JSHint_Test.java

@@ -83,7 +83,12 @@ public void checkWithoutLoad() {
 
   @Test( expected = NullPointerException.class )
   public void checkWithNullCode() {
-    jsHint.check( null, handler );
+    jsHint.check( (String)null, handler );
+  }
+
+  @Test( expected = NullPointerException.class )
+  public void checkWithNullText() {
+    jsHint.check( (Text)null, handler );
   }
 
   @Test

com.eclipsesource.jshint.test/src/com/eclipsesource/jshint/Text_Test.java

@@ -10,6 +10,7 @@
  ******************************************************************************/
 package com.eclipsesource.jshint;
 
+import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
 
@@ -24,6 +25,24 @@
 
 public class Text_Test {
 
+  @Test( expected = NullPointerException.class )
+  public void createWithNullStringFails() {
+    new Text( (String)null );
+  }
+
+  @Test( expected = NullPointerException.class )
+  public void createWithNullReaderFails() throws IOException {
+    new Text( (Reader)null );
+  }
+
+  @Test
+  public void createWithString() {
+    Text textFile = new Text( "" );
+
+    assertEquals( "", textFile.getContent() );
+    assertEquals( 1, textFile.getLineCount() );
+  }
+
   @Test
   public void emptyString() throws Exception {
     Reader reader = new StringReader( "" );

com.eclipsesource.jshint.test/src/com/eclipsesource/jshint/internal/JSHintRunner_Test.java

@@ -148,15 +148,15 @@ public void illegalCharset() throws Exception {
   public void customLibrary() throws Exception {
     JSHintRunner runner = new JSHintRunner();
     String fakeJsHint = "JSHINT = function() { return false; };"
-            + "JSHINT.errors = [ { line: 23, character: 42, reason: 'test' } ]";
+            + "JSHINT.errors = [ { line: 1, character: 2, reason: 'test' } ]";
     File fakeJSHintFile = createTmpFile( fakeJsHint, "UTF-8" );
     File jsFile = createTmpFile( "-- ignored --", "UTF-8" );
     String fakeJSHintFileName = fakeJSHintFile.getAbsolutePath();
     String jsFileName = jsFile.getAbsolutePath();
 
     runner.run( "--custom", fakeJSHintFileName, jsFileName );
 
-    assertThat( getSysout(), startsWith( "Problem in file " + jsFileName + " at line 23: test" ) );
+    assertThat( getSysout(), startsWith( "Problem in file " + jsFileName + " at line 1: test" ) );
   }
 
   @Test

com.eclipsesource.jshint.ui.test/src/com/eclipsesource/jshint/ui/internal/preferences/OptionParserUtil_Test.java

@@ -26,16 +26,14 @@ public class OptionParserUtil_Test {
   public void createConfiguration() {
     Configuration result = OptionParserUtil.createConfiguration( "opt1: true", "predef1: false" );
 
-    String expected = "{\"predef\": {\"predef1\": false}, \"indent\": 1, \"opt1\": true}";
-    assertEquals( expected, result.toJson() );
+    assertEquals( "{\"predef\": {\"predef1\": false}, \"opt1\": true}", result.toJson() );
   }
 
   @Test
   public void createConfiguration_withEmptyParameters() {
     Configuration result = OptionParserUtil.createConfiguration( "", "" );
 
-    String expected = "{\"indent\": 1}";
-    assertEquals( expected, result.toJson() );
+    assertEquals( "{}", result.toJson() );
   }
 
   @Test( expected = NullPointerException.class )

com.eclipsesource.jshint.ui/src/com/eclipsesource/jshint/ui/internal/builder/JSHintBuilderVisitor.java

@@ -101,7 +101,7 @@ private void check( IFile file ) throws CoreException {
     Text code = readContent( file );
     ProblemHandler handler = new MarkerHandler( new MarkerAdapter( file ), code );
     try {
-      checker.check( code.getContent(), handler );
+      checker.check( code, handler );
     } catch( CoreExceptionWrapper wrapper ) {
       throw (CoreException)wrapper.getCause();
     } catch( RuntimeException exception ) {

com.eclipsesource.jshint/src/com/eclipsesource/jshint/Configuration.java

@@ -27,7 +27,6 @@ public class Configuration {
   public Configuration() {
     predefs = new LinkedHashMap<String, Object>();
     options = new LinkedHashMap<String, Object>();
-    options.put( "indent", Integer.valueOf( 1 ) );
   }
 
   /**
@@ -78,6 +77,10 @@ public String toJson() {
     return builder.toString();
   }
 
+  Object getOption( String option ) {
+    return options.get( option );
+  }
+
   private void addMap( StringBuilder builder, Map<String, Object> map ) {
     boolean first = true;
     for( String key : map.keySet() ) {

com.eclipsesource.jshint/src/com/eclipsesource/jshint/JSHint.java

@@ -46,9 +46,11 @@
 public class JSHint {
 
   private static final String DEFAULT_JSHINT_VERSION = "r12";
+  private static final int DEFAULT_JSHINT_INDENT = 4;
   private Function jshint;
   private Object opts;
   private ScriptableObject scope;
+  private int indent = DEFAULT_JSHINT_INDENT;
 
   /**
    * Loads the default JSHint library.
@@ -101,11 +103,20 @@ public void configure( Configuration configuration ) {
       ScriptableObject scope = context.initStandardObjects();
       String optionsString = configuration.toJson();
       opts = context.evaluateString( scope, "opts = " + optionsString + ";", "[options]", 1, null );
+      indent = determineIndent( configuration );
     } finally {
       Context.exit();
     }
   }
 
+  private int determineIndent( Configuration configuration ) {
+    Object indent = configuration.getOption( "indent" );
+    if( indent instanceof Integer ) {
+      return ( (Integer)indent ).intValue();
+    }
+    return DEFAULT_JSHINT_INDENT;
+  }
+
   /**
    * Checks the given JavaScript code. All problems will be reported to the given problem handler.
    *
@@ -119,18 +130,26 @@ public boolean check( String code, ProblemHandler handler ) {
     if( code == null ) {
       throw new NullPointerException( "code is null" );
     }
+    return check( new Text( code ), handler );
+  }
+
+  public boolean check( Text text, ProblemHandler handler ) {
+    if( text == null ) {
+      throw new NullPointerException( "code is null" );
+    }
     if( jshint == null ) {
       throw new IllegalStateException( "JSHint is not loaded" );
     }
     boolean result = true;
+    String code = text.getContent();
     // Don't feed jshint with empty strings, see https://github.com/jshint/jshint/issues/615
     // However, consider an empty string valid
     if( code.trim().length() != 0 ) {
       Context context = Context.enter();
       try {
         result = checkCode( context, code );
         if( !result && handler != null ) {
-          handleProblems( handler );
+          handleProblems( handler, text );
         }
       } finally {
         Context.exit();
@@ -187,30 +206,45 @@ private Function findJSHintFunction( ScriptableObject scope ) throws IllegalArgu
     return (Function)object;
   }
 
-  private void handleProblems( ProblemHandler handler ) {
+  private void handleProblems( ProblemHandler handler, Text text ) {
     NativeArray errors = (NativeArray)jshint.get( "errors", jshint );
     long length = errors.getLength();
     for( int i = 0; i < length; i++ ) {
       Object object = errors.get( i, errors );
       ScriptableObject error = (ScriptableObject)object;
       if( error != null ) {
-        Problem problem = createProblem( error );
+        Problem problem = createProblem( error, text );
         handler.handleProblem( problem );
       }
     }
   }
 
-  private ProblemImpl createProblem( ScriptableObject error ) {
+  private ProblemImpl createProblem( ScriptableObject error, Text text ) {
     String reason = getPropertyAsString( error, "reason", "" );
     int line = getPropertyAsInt( error, "line", -1 );
     int character = getPropertyAsInt( error, "character", -1 );
     if( character > 0 ) {
-      character -= 1;
+      character = fixPosition( text, line, character );
     }
     String message = reason.endsWith( "." ) ? reason.substring( 0, reason.length() - 1 ) : reason;
     return new ProblemImpl( line, character, message );
   }
 
+  private int fixPosition( Text text, int line, int character ) {
+    // JSHint reports physical character positions instead of a character index,
+    // i.e. every tab character is multiplied with the indent.
+    String string = text.getContent();
+    int offset = text.getLineOffset( line - 1 );
+    int indentIndex = 0;
+    int charIndex = 0;
+    while( indentIndex < character - 1 ) {
+      boolean isTab = string.charAt( offset + indentIndex ) == '\t';
+      indentIndex += isTab ? indent : 1;
+      charIndex++;
+    }
+    return charIndex;
+  }
+
   private static String getPropertyAsString( ScriptableObject object,
                                              String name,
                                              String defaultValue )

com.eclipsesource.jshint/src/com/eclipsesource/jshint/Text.java

@@ -12,6 +12,7 @@
 
 import java.io.IOException;
 import java.io.Reader;
+import java.io.StringReader;
 
 
 /**
@@ -23,7 +24,24 @@ public class Text {
   private int lineCount = 1;
   private int[] lineOffsets = new int[ 200 ];
 
+  public Text( String text ) {
+    if( text == null ) {
+      throw new NullPointerException( "text is null" );
+    }
+    StringReader reader = new StringReader( text );
+    try {
+      read( reader );
+    } catch( IOException exception ) {
+      throw new RuntimeException( exception );
+    } finally {
+      reader.close();
+    }
+  }
+
   public Text( Reader reader ) throws IOException {
+    if( reader == null ) {
+      throw new NullPointerException( "reader is null" );
+    }
     read( reader );
   }
 
@@ -44,7 +62,7 @@ public int getLineCount() {
    */
   public int getLineOffset( int line ) {
     if( line >= lineCount ) {
-      throw new IndexOutOfBoundsException( "line does not exist" );
+      throw new IndexOutOfBoundsException( "line does not exist: " + line );
     }
     return lineOffsets[ line ];
   }