Filter comments from JSHint config in workspace prefs

The CommentsFilter had been added in commit 8ed2bdc, but enabled only
for project-specific configs, not for the config in the workspace
preferences.

Fixes #76

Author: ralfstx

bundles/com.eclipsesource.jshint.ui/src/com/eclipsesource/jshint/ui/internal/builder/CommentsFilter.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 EclipseSource and others.
+ * Copyright (c) 2013, 2014 EclipseSource and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,7 +11,6 @@
 package com.eclipsesource.jshint.ui.internal.builder;
 
 
-
 public class CommentsFilter {
 
   private final char[] chars;
@@ -56,4 +55,8 @@ public String toString() {
     return new String( chars );
   }
 
+  public static String filterComments( String input ) {
+    return new CommentsFilter( input ).toString();
+  }
+
 }

bundles/com.eclipsesource.jshint.ui/src/com/eclipsesource/jshint/ui/internal/builder/ConfigLoader.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 EclipseSource and others.
+ * Copyright (c) 2013, 2014 EclipseSource and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -10,16 +10,21 @@
  ******************************************************************************/
 package com.eclipsesource.jshint.ui.internal.builder;
 
+import java.io.IOException;
+
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
+import org.eclipse.core.runtime.CoreException;
 import org.osgi.service.prefs.Preferences;
 
 import com.eclipsesource.jshint.ui.internal.Activator;
 import com.eclipsesource.jshint.ui.internal.preferences.OptionsPreferences;
 import com.eclipsesource.jshint.ui.internal.preferences.PreferencesFactory;
-import com.eclipsesource.jshint.ui.internal.util.IOUtil;
 import com.eclipsesource.json.JsonObject;
 
+import static com.eclipsesource.jshint.ui.internal.builder.CommentsFilter.filterComments;
+import static com.eclipsesource.jshint.ui.internal.util.IOUtil.readFileUtf8;
+
 
 public class ConfigLoader {
 
@@ -38,34 +43,46 @@ public JsonObject getConfiguration() {
     return getWorkspaceConfig();
   }
 
-  private static JsonObject getWorkspaceConfig() {
-    Preferences workspaceNode = PreferencesFactory.getWorkspacePreferences();
-    return JsonObject.readFrom( new OptionsPreferences( workspaceNode ).getConfig() );
+  private JsonObject getProjectConfig( OptionsPreferences projectPrefs ) {
+    try {
+      String json = getProjectConfigJson( projectPrefs );
+      return JsonObject.readFrom( filterComments( json ) );
+    } catch( Exception exception ) {
+      String message = "Failed to read jshint configuration for project " + project.getName();
+      Activator.logError( message, exception );
+      return new JsonObject();
+    }
   }
 
-  private JsonObject getProjectConfig( OptionsPreferences projectPrefs ) {
-    IFile configFile = getConfigFile();
-    // compatibility
+  private String getProjectConfigJson( OptionsPreferences projectPrefs ) throws CoreException,
+      IOException
+  {
+    IFile configFile = getProjectConfigFile();
     if( !configFile.exists() ) {
-      return JsonObject.readFrom( projectPrefs.getConfig() );
+      // compatibility
+      return projectPrefs.getConfig();
     }
-    return readConfig( configFile );
+    return readFileUtf8( configFile );
   }
 
-  private IFile getConfigFile() {
+  private IFile getProjectConfigFile() {
     return project.getFile( ".jshintrc" );
   }
 
-  private JsonObject readConfig( IFile file ) {
+  private static JsonObject getWorkspaceConfig() {
     try {
-      String contents = IOUtil.readFileUtf8( file );
-      String filtered = new CommentsFilter( contents ).toString();
-      return JsonObject.readFrom( filtered );
+      String json = getWorkspaceConfigJson();
+      return JsonObject.readFrom( filterComments( json ) );
     } catch( Exception exception ) {
-      String message = "Failed to read jshint configuration for project " + project.getName();
+      String message = "Failed to read jshint configuration from workspace preferences";
       Activator.logError( message, exception );
+      return new JsonObject();
     }
-    return new JsonObject();
+  }
+
+  private static String getWorkspaceConfigJson() {
+    Preferences workspaceNode = PreferencesFactory.getWorkspacePreferences();
+    return new OptionsPreferences( workspaceNode ).getConfig();
   }
 
 }

tests/com.eclipsesource.jshint.ui.test/src/com/eclipsesource/jshint/ui/internal/builder/ConfigLoader_Test.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 EclipseSource.
+ * Copyright (c) 2013, 2014 EclipseSource and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -10,10 +10,13 @@
  ******************************************************************************/
 package com.eclipsesource.jshint.ui.internal.builder;
 
+import java.util.Arrays;
+
 import org.eclipse.core.resources.IProject;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.osgi.service.prefs.BackingStoreException;
 
 import com.eclipsesource.jshint.ui.internal.preferences.OptionsPreferences;
 import com.eclipsesource.jshint.ui.internal.preferences.PreferencesFactory;
@@ -39,8 +42,9 @@ public void setUp() {
   }
 
   @After
-  public void tearDown() {
+  public void tearDown() throws BackingStoreException {
     deleteProject( project );
+    PreferencesFactory.getWorkspacePreferences().clear();
   }
 
   @Test
@@ -56,7 +60,7 @@ public void usesWorkspaceOptionsByDefault() {
   }
 
   @Test
-  public void ignoresWorkspaceOptions_ifProjectSpecific() {
+  public void ignoresWorkspaceOptionsIfProjectSpecific() {
     workspacePrefs.getNode().put( "options", "a: 1, b: 1" );
     createProjectConfig( new JsonObject().add( "b", 2 ).add( "c", 2 ) );
     projectPrefs.setProjectSpecific( true );
@@ -68,6 +72,18 @@ public void ignoresWorkspaceOptions_ifProjectSpecific() {
     assertEquals( 2, configuration.get( "c" ).asInt() );
   }
 
+  @Test
+  public void usesWorkspaceConfigIfNotProjectSpecific() {
+    workspacePrefs.setConfig( "{\"a\": 1, \"b\": 1}" );
+    createProjectConfig( new JsonObject().add( "b", 2 ).add( "c", 2 ) );
+
+    JsonObject configuration = new ConfigLoader( project ).getConfiguration();
+
+    assertEquals( 1, configuration.get( "a" ).asInt() );
+    assertEquals( 1, configuration.get( "b" ).asInt() );
+    assertNull( configuration.get( "c" ) );
+  }
+
   @Test
   public void fallsBackToOldProjectProperties_ifConfigFileMissing() {
     projectPrefs.setProjectSpecific( true );
@@ -102,6 +118,27 @@ public void emptyConfigForProjectsWithoutConfigFileAndProperties() {
     assertEquals( new JsonObject(), configuration );
   }
 
+  @Test
+  public void filtersCommentsFromProjectConfig() {
+    projectPrefs.setConfig( "{\n// \"a\": 1,\n\"b\": 2 /*, \"c\": 3*/}" );
+    projectPrefs.setProjectSpecific( true );
+
+    JsonObject configuration = new ConfigLoader( project ).getConfiguration();
+
+    assertEquals( new JsonObject().add( "b", 2 ), configuration );
+    assertEquals( Arrays.asList( "b" ), configuration.names() );
+  }
+
+  @Test
+  public void filtersCommentsFromWorkspaceConfig() {
+    workspacePrefs.setConfig( "{\n// \"a\": 1,\n\"b\": 2 /*, \"c\": 3*/}" );
+
+    JsonObject configuration = new ConfigLoader( project ).getConfiguration();
+
+    assertEquals( new JsonObject().add( "b", 2 ), configuration );
+    assertEquals( Arrays.asList( "b" ), configuration.names() );
+  }
+
   private void createProjectConfig( JsonObject projectConfig ) {
     TestUtil.createFile( project, ".jshintrc", projectConfig.toString() );
   }