From a3f8ffd001bcaa10a5f9595484a965270ad21f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20M=C3=BCller?= Date: Sat, 26 Apr 2014 21:42:18 +0200 Subject: [PATCH] fixed bug with duplicated resources for package objects on first level of hierarchy --- .../plugins/scala/language/ScalaFile.java | 41 +++-- .../plugins/scala/language/ScalaPackage.java | 14 +- .../sensor/ScalaSourceImporterSensor.java | 19 +-- .../plugins/scala/compiler/Compiler.scala | 4 +- .../plugins/scala/language/ScalaFileTest.java | 27 ++- .../scala/sensor/FakeSensorContext.java | 160 ++++++++++++++++++ .../sensor/ScalaSourceImporterSensorTest.java | 45 +++-- .../plugins/scala/util/FileTestUtils.java | 29 ++-- .../package.scala | 5 + .../otherFolder/package.scala | 5 + .../scalaSourceImporter/package.scala | 5 + 11 files changed, 285 insertions(+), 69 deletions(-) create mode 100644 src/test/java/org/sonar/plugins/scala/sensor/FakeSensorContext.java create mode 100644 src/test/resources/otherFolderForPackageObjectOnFirstLevel/package.scala create mode 100644 src/test/resources/scalaSourceImporter/otherFolder/package.scala create mode 100644 src/test/resources/scalaSourceImporter/package.scala diff --git a/src/main/java/org/sonar/plugins/scala/language/ScalaFile.java b/src/main/java/org/sonar/plugins/scala/language/ScalaFile.java index 050a63d..05d8bfc 100644 --- a/src/main/java/org/sonar/plugins/scala/language/ScalaFile.java +++ b/src/main/java/org/sonar/plugins/scala/language/ScalaFile.java @@ -116,7 +116,7 @@ public static ScalaFile fromInputFile(InputFile inputFile) { /** * Creates a {@link ScalaFile} from a file in the source directories. * - * @param inputFile the file object with relative path + * @param inputFile the file object with relative path * @param isUnitTest whether it is a unit test file or a source file * @return the {@link ScalaFile} created if exists, null otherwise */ @@ -125,12 +125,27 @@ public static ScalaFile fromInputFile(InputFile inputFile, boolean isUnitTest) { return null; } - final String packageName = PackageResolver.resolvePackageNameOfFile( - inputFile.getFile().getAbsolutePath()); - final String className = resolveClassName(inputFile); + String packageName = PackageResolver.resolvePackageNameOfFile(inputFile.getFile().getAbsolutePath()); + String className = resolveClassName(inputFile); + + if (isPackageObjectInFirstLevelPackage(packageName, className)) { + String lastFolderName = extractLastFolderName(inputFile); + return new ScalaFile(StringUtils.EMPTY, lastFolderName + "." + className, isUnitTest); + } + return new ScalaFile(packageName, className, isUnitTest); } + private static boolean isPackageObjectInFirstLevelPackage(String packageName, String className) { + return "".equalsIgnoreCase(packageName) && "package".equalsIgnoreCase(className); + } + + private static String extractLastFolderName(InputFile inputFile) { + String absolutePath = inputFile.getFile().getAbsolutePath(); + int lastPathSeparator = absolutePath.lastIndexOf("/"); + return absolutePath.substring(absolutePath.lastIndexOf("/", lastPathSeparator - 1) + 1, lastPathSeparator); + } + private static String resolveClassName(InputFile inputFile) { String classname = inputFile.getRelativePath(); if (inputFile.getRelativePath().indexOf('/') >= 0) { @@ -139,12 +154,14 @@ private static String resolveClassName(InputFile inputFile) { return StringUtils.substringBeforeLast(classname, "."); } - @Override - public String toString() { - return new ToStringBuilder(this) - .append("key", getKey()) - .append("fileName", filename) - .append("isUnitTest", isUnitTest) - .toString(); - } + @Override + public String toString() { + return new ToStringBuilder(this) + .append("key", getKey()) + .append("fileName", filename) + .append("isUnitTest", isUnitTest) + .append("longName", longName) + .append("parent", parent) + .toString(); + } } diff --git a/src/main/java/org/sonar/plugins/scala/language/ScalaPackage.java b/src/main/java/org/sonar/plugins/scala/language/ScalaPackage.java index 23df2f9..ea03792 100644 --- a/src/main/java/org/sonar/plugins/scala/language/ScalaPackage.java +++ b/src/main/java/org/sonar/plugins/scala/language/ScalaPackage.java @@ -20,6 +20,7 @@ package org.sonar.plugins.scala.language; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.builder.ToStringBuilder; import org.sonar.api.resources.Language; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Resource; @@ -37,10 +38,6 @@ public class ScalaPackage extends Resource { public static final String DEFAULT_PACKAGE_NAME = "[default]"; - public ScalaPackage() { - this(null); - } - public ScalaPackage(String key) { super(); setKey(StringUtils.defaultIfEmpty(StringUtils.trim(key), DEFAULT_PACKAGE_NAME)); @@ -87,4 +84,11 @@ public boolean matchFilePattern(String antPattern) { WildcardPattern matcher = WildcardPattern.create(patternWithoutFileSuffix, "."); return matcher.match(getKey()); } -} \ No newline at end of file + + @Override + public String toString() { + return new ToStringBuilder(this) + .append("key", getKey()) + .toString(); + } +} diff --git a/src/main/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensor.java b/src/main/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensor.java index b94f219..9785534 100644 --- a/src/main/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensor.java +++ b/src/main/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensor.java @@ -19,8 +19,6 @@ */ package org.sonar.plugins.scala.sensor; -import java.io.IOException; - import org.apache.commons.io.FileUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,6 +31,8 @@ import org.sonar.plugins.scala.language.Scala; import org.sonar.plugins.scala.language.ScalaFile; +import java.io.IOException; + /** * This Sensor imports all Scala files into Sonar. * @@ -61,20 +61,19 @@ public void analyse(Project project, SensorContext sensorContext) { } } - private void addFileToSonar(SensorContext sensorContext, InputFile inputFile, - boolean isUnitTest, String charset) { + private void addFileToSonar(SensorContext sensorContext, InputFile inputFile, boolean isUnitTest, String charset) { try { String source = FileUtils.readFileToString(inputFile.getFile(), charset); - ScalaFile resource = ScalaFile.fromInputFile(inputFile, isUnitTest); + ScalaFile file = ScalaFile.fromInputFile(inputFile, isUnitTest); - sensorContext.index(resource); - sensorContext.saveSource(resource, source); + sensorContext.index(file); + sensorContext.saveSource(file, source); if (LOGGER.isDebugEnabled()) { if (isUnitTest) { - LOGGER.debug("Added Scala test file to Sonar: " + inputFile.getFile().getAbsolutePath()); + LOGGER.debug("Added Scala test file to Sonar: {}", file); } else { - LOGGER.debug("Added Scala source file to Sonar: " + inputFile.getFile().getAbsolutePath()); + LOGGER.debug("Added Scala source file to Sonar: {}", file); } } } catch (IOException ioe) { @@ -86,4 +85,4 @@ private void addFileToSonar(SensorContext sensorContext, InputFile inputFile, public String toString() { return getClass().getSimpleName(); } -} \ No newline at end of file +} diff --git a/src/main/scala/org/sonar/plugins/scala/compiler/Compiler.scala b/src/main/scala/org/sonar/plugins/scala/compiler/Compiler.scala index 115483e..d6411ef 100644 --- a/src/main/scala/org/sonar/plugins/scala/compiler/Compiler.scala +++ b/src/main/scala/org/sonar/plugins/scala/compiler/Compiler.scala @@ -35,12 +35,12 @@ object Compiler extends Global(new Settings() { }) { - private val LOGGER = LoggerFactory.getLogger(classOf[Compiler]) + private val logger = LoggerFactory.getLogger(classOf[Compiler]) try { new Run() } catch { - case ex: Throwable => LOGGER.error("Could not initiate Scala compiler, probably due classpath issues!", ex) + case ex: Throwable => logger.error("Could not initiate Scala compiler, probably due classpath issues!", ex) } override def forScaladoc = true diff --git a/src/test/java/org/sonar/plugins/scala/language/ScalaFileTest.java b/src/test/java/org/sonar/plugins/scala/language/ScalaFileTest.java index dfb0c21..8c440d7 100644 --- a/src/test/java/org/sonar/plugins/scala/language/ScalaFileTest.java +++ b/src/test/java/org/sonar/plugins/scala/language/ScalaFileTest.java @@ -19,6 +19,14 @@ */ package org.sonar.plugins.scala.language; +import org.junit.Test; +import org.sonar.api.resources.InputFile; +import org.sonar.api.resources.InputFileUtils; +import org.sonar.api.resources.Qualifiers; +import org.sonar.plugins.scala.util.FileTestUtils; + +import java.io.File; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertNull; @@ -26,13 +34,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.io.File; - -import org.junit.Test; -import org.sonar.api.resources.InputFile; -import org.sonar.api.resources.Qualifiers; -import org.sonar.plugins.scala.util.FileTestUtils; - public class ScalaFileTest { @Test @@ -71,6 +72,16 @@ public void shouldCreateScalaTestFileWithCorrectAttributes() { assertThat(scalaFile.isUnitTest(), is(true)); } + @Test + public void shouldHandlePackeObjectsInFirstLevelProperly() { + InputFile inputFile = InputFileUtils.create(new File("src/test/resources/"), "scalaSourceImporter/package.scala"); + ScalaFile scalaFile = ScalaFile.fromInputFile(inputFile, false); + + assertThat(scalaFile.getName(), is("scalaSourceImporter.package")); + assertThat(scalaFile.getLongName(), is(scalaFile.getName())); + assertThat(scalaFile.getKey(), is("[default].scalaSourceImporter.package")); + } + @Test public void shouldNotCreateScalaFileIfInputFileIsNull() { assertNull(ScalaFile.fromInputFile(null)); @@ -90,4 +101,4 @@ public void shouldNotCreateScalaFileIfRelativePathIsNull() { when(inputFile.getRelativePath()).thenReturn(null); assertNull(ScalaFile.fromInputFile(inputFile)); } -} \ No newline at end of file +} diff --git a/src/test/java/org/sonar/plugins/scala/sensor/FakeSensorContext.java b/src/test/java/org/sonar/plugins/scala/sensor/FakeSensorContext.java new file mode 100644 index 0000000..d829ed0 --- /dev/null +++ b/src/test/java/org/sonar/plugins/scala/sensor/FakeSensorContext.java @@ -0,0 +1,160 @@ +/* + * Sonar Scala Plugin + * Copyright (C) 2011 - 2014 All contributors + * dev@sonar.codehaus.org + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.scala.sensor; + +import com.google.common.collect.Sets; +import org.sonar.api.batch.Event; +import org.sonar.api.batch.SensorContext; +import org.sonar.api.design.Dependency; +import org.sonar.api.measures.Measure; +import org.sonar.api.measures.MeasuresFilter; +import org.sonar.api.measures.Metric; +import org.sonar.api.resources.ProjectLink; +import org.sonar.api.resources.Resource; +import org.sonar.api.rules.Violation; + +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Set; + +/** + * This fake helps to detect duplicate resources in tests. + */ +class FakeSensorContext implements SensorContext { + + private final Set resources = Sets.newHashSet(); + + public void saveSource(Resource reference, String source) { + if (resources.contains(reference)) { + throw new RuntimeException("Duplicate resources in sensor context are not allowed!"); + } + resources.add(reference); + } + + public boolean index(Resource resource) { + return false; + } + + public boolean index(Resource resource, Resource parentReference) { + return false; + } + + public boolean isExcluded(Resource reference) { + return false; + } + + public boolean isIndexed(Resource reference, boolean acceptExcluded) { + return false; + } + + public R getResource(R reference) { + return null; + } + + public Resource getParent(Resource reference) { + return null; + } + + public Collection getChildren(Resource reference) { + return null; + } + + public Measure getMeasure(Metric metric) { + return null; + } + + public M getMeasures(MeasuresFilter filter) { + return null; + } + + public Measure saveMeasure(Measure measure) { + return null; + } + + public Measure saveMeasure(Metric metric, Double value) { + return null; + } + + public Measure getMeasure(Resource resource, Metric metric) { + return null; + } + + public String saveResource(Resource resource) { + return null; + } + + public M getMeasures(Resource resource, MeasuresFilter filter) { + return null; + } + + public Measure saveMeasure(Resource resource, Metric metric, Double value) { + return null; + } + + public Measure saveMeasure(Resource resource, Measure measure) { + return null; + } + + public void saveViolation(Violation violation, boolean force) { + + } + + public void saveViolation(Violation violation) { + + } + + public void saveViolations(Collection violations) { + + } + + public Dependency saveDependency(Dependency dependency) { + return null; + } + + public Set getDependencies() { + return null; + } + + public Collection getIncomingDependencies(Resource to) { + return null; + } + + public Collection getOutgoingDependencies(Resource from) { + return null; + } + + public void saveLink(ProjectLink link) { + } + + public void deleteLink(String key) { + } + + public List getEvents(Resource resource) { + return null; + } + + public Event createEvent(Resource resource, String name, String description, String category, Date date) { + return null; + } + + public void deleteEvent(Event event) { + } +} diff --git a/src/test/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensorTest.java b/src/test/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensorTest.java index 7c39913..49ec744 100644 --- a/src/test/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensorTest.java +++ b/src/test/java/org/sonar/plugins/scala/sensor/ScalaSourceImporterSensorTest.java @@ -19,31 +19,28 @@ */ package org.sonar.plugins.scala.sensor; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.List; - import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; import org.sonar.api.batch.SensorContext; import org.sonar.api.resources.InputFile; +import org.sonar.api.resources.InputFileUtils; import org.sonar.api.resources.Java; import org.sonar.api.resources.Project; import org.sonar.api.resources.ProjectFileSystem; import org.sonar.plugins.scala.language.Scala; import org.sonar.plugins.scala.util.FileTestUtils; +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.*; + public class ScalaSourceImporterSensorTest { private ScalaSourceImporterSensor scalaSourceImporter; @@ -62,7 +59,7 @@ public void setUp() { project = mock(Project.class); when(project.getFileSystem()).thenReturn(fileSystem); - sensorContext = mock(SensorContext.class); + sensorContext = spy(new FakeSensorContext()); } @Test @@ -201,6 +198,22 @@ public void shouldImportAllScalaTestFilesWithTheCorrectFileContent() throws IOEx verifyNoMoreInteractions(sensorContext); } + @Test + public void shouldImportAllPackageObjects() { + when(fileSystem.mainFiles(scalaSourceImporter.getScala().getKey())).thenReturn(java.util.Arrays.asList( + InputFileUtils.create(new File("src/test/resources/"), "scalaSourceImporter/package.scala"), + InputFileUtils.create(new File("src/test/resources/"), "scalaSourceImporter/otherFolder/package.scala"), + InputFileUtils.create(new File("src/test/resources/"), "otherFolderForPackageObjectOnFirstLevel/package.scala") + )); + when(fileSystem.testFiles(scalaSourceImporter.getScala().getKey())).thenReturn(new ArrayList()); + + scalaSourceImporter.analyse(project, sensorContext); + + verify(sensorContext, times(3)).index(eq(FileTestUtils.SCALA_SOURCE_FILE)); + verify(sensorContext, times(3)).saveSource(eq(FileTestUtils.SCALA_SOURCE_FILE), any(String.class)); + verifyNoMoreInteractions(sensorContext); + } + public List getInputFiles(int numberOfFiles) { return FileTestUtils.getInputFiles("/scalaSourceImporter/", "MainFile", numberOfFiles); } @@ -216,4 +229,4 @@ public List getContentOfFiles(int numberOfFiles) throws IOException { public List getContentOfTestFiles(int numberOfFiles) throws IOException { return FileTestUtils.getContentOfFiles("/scalaSourceImporter/", "TestFile", numberOfFiles); } -} \ No newline at end of file +} diff --git a/src/test/java/org/sonar/plugins/scala/util/FileTestUtils.java b/src/test/java/org/sonar/plugins/scala/util/FileTestUtils.java index 723a77f..7f93b33 100644 --- a/src/test/java/org/sonar/plugins/scala/util/FileTestUtils.java +++ b/src/test/java/org/sonar/plugins/scala/util/FileTestUtils.java @@ -19,13 +19,6 @@ */ package org.sonar.plugins.scala.util; -import java.io.File; -import java.io.IOException; -import java.net.URL; -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.List; - import org.apache.commons.io.FileUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,6 +26,13 @@ import org.sonar.api.resources.InputFileUtils; import org.sonar.plugins.scala.language.ScalaFile; +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; + public final class FileTestUtils { public static final ScalaFile SCALA_SOURCE_FILE = new DummyScalaFile(false); @@ -52,12 +52,11 @@ public static List getInputFiles(String path, String fileNameBase, in return getInputFiles(path, fileNameBase, "scala", numberOfFiles); } - public static List getInputFiles(String path, String fileNameBase, - String fileSuffix, int numberOfFiles) { + public static List getInputFiles(String path, String fileNameBase, String fileSuffix, int numberOfFiles) { List mainFiles = new ArrayList(); URL resourceURL = FileTestUtils.class.getResource(path + fileNameBase + "1." + fileSuffix); - for (int i = 1; resourceURL != null && i <= numberOfFiles;) { + for (int i = 1; resourceURL != null && i <= numberOfFiles; ) { mainFiles.add(new File(resourceURL.getFile())); resourceURL = FileTestUtils.class.getResource(path + fileNameBase + (++i) + "." + fileSuffix); } @@ -65,15 +64,13 @@ public static List getInputFiles(String path, String fileNameBase, return InputFileUtils.create(new File(FileTestUtils.class.getResource(path).getFile()), mainFiles); } - public static List getContentOfFiles(String path, String fileNameBase, - int numberOfFiles) throws IOException { + public static List getContentOfFiles(String path, String fileNameBase, int numberOfFiles) throws IOException { List contentOfFiles = new ArrayList(); URL resourceURL = FileTestUtils.class.getResource(path + fileNameBase + "1.scala"); - for (int i = 1; resourceURL != null && i <= numberOfFiles;) { + for (int i = 1; resourceURL != null && i <= numberOfFiles; ) { try { - contentOfFiles.add(FileUtils.readFileToString(new File(resourceURL.getFile()), - Charset.defaultCharset().toString())); + contentOfFiles.add(FileUtils.readFileToString(new File(resourceURL.getFile()), Charset.defaultCharset().toString())); } catch (IOException ioe) { LOGGER.error("Unexpected I/O exception occurred", ioe); throw ioe; @@ -83,4 +80,4 @@ public static List getContentOfFiles(String path, String fileNameBase, return contentOfFiles; } -} \ No newline at end of file +} diff --git a/src/test/resources/otherFolderForPackageObjectOnFirstLevel/package.scala b/src/test/resources/otherFolderForPackageObjectOnFirstLevel/package.scala new file mode 100644 index 0000000..10aad8b --- /dev/null +++ b/src/test/resources/otherFolderForPackageObjectOnFirstLevel/package.scala @@ -0,0 +1,5 @@ +import java.util.Collections + +package object otherFolderForPackageObjectOnFirstLevel { + +} diff --git a/src/test/resources/scalaSourceImporter/otherFolder/package.scala b/src/test/resources/scalaSourceImporter/otherFolder/package.scala new file mode 100644 index 0000000..1b5a8e2 --- /dev/null +++ b/src/test/resources/scalaSourceImporter/otherFolder/package.scala @@ -0,0 +1,5 @@ +package scalaSourceImporter + +package object otherFolder { + +} diff --git a/src/test/resources/scalaSourceImporter/package.scala b/src/test/resources/scalaSourceImporter/package.scala new file mode 100644 index 0000000..f9002bb --- /dev/null +++ b/src/test/resources/scalaSourceImporter/package.scala @@ -0,0 +1,5 @@ +import java.util.Collections + +package object scalaSourceImporter { + +}