support 6.7 LTS #840

vilchik-elena · 2017-11-24T12:29:20Z

Fixes #837, fixes #839

SonarTech · 2017-11-27T15:31:53Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptSquidSensor.java

    Collection<File> files = StreamSupport.stream(inputFiles.spliterator(), false)
-      .map(CompatibleInputFile::file)
+      .map(InputFile::file)


Remove this use of "file"; it is deprecated.

inverno · 2017-11-28T08:39:10Z

javascript-checks-testkit/src/main/java/org/sonar/javascript/checks/verifier/TestUtils.java

    } catch (IOException e) {
      throw Throwables.propagate(e);
    }
  }

-  private static Settings settings() {
-    Settings settings = new MapSettings();
+  private static Configuration config() {


@vilchik-elena just a question: this kind of change is not related to dropping old versions support, it's about preparing for future removals of currently deprecated code, right?

@inverno mostly yes (while note, that MapSettings changed its package)

inverno · 2017-11-28T10:58:48Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java

+        .defaultValue(LCOV_REPORT_PATHS_DEFAULT_VALUE)
+        .name("LCOV Files")
+        .description("Paths (absolute or relative) to the files with LCOV data.")
+        .onQualifiers(Qualifiers.MODULE, Qualifiers.PROJECT)


@vilchik-elena add multivalues

inverno · 2017-11-28T11:00:12Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java

+        .subCategory(TEST_AND_COVERAGE)
+        .category(JAVASCRIPT_CATEGORY)
+        .build(),
+
      PropertyDefinition.builder(FILE_SUFFIXES_KEY)
        .defaultValue(FILE_SUFFIXES_DEFVALUE)
        .name("File Suffixes")
        .description("Comma-separated list of suffixes for files to analyze.")


@vilchik-elena maybe drop Comma-separated here as well

inverno · 2017-11-28T11:09:50Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptSquidSensor.java

@@ -177,13 +162,13 @@ private void analyse(SensorContext sensorContext, CompatibleInputFile inputFile,
      scanFile(sensorContext, inputFile, executor, visitors, scriptTree);
    } catch (RecognitionException e) {
      checkInterrupted(e);
-      LOG.error("Unable to parse file: " + inputFile.absolutePath());
+      LOG.error("Unable to parse file: " + inputFile.toString());


@vilchik-elena I would use inputFile.uri(). It might be more stable than toString and it is absolute, as before, unlike the current implementation of DefaultInputFile.toString() which is using the project-relative path. Unless the goal was to use the project-relative path, in which case we should use InputFile.getProjectRelativePath().

inverno · 2017-11-28T11:10:11Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptSquidSensor.java

      LOG.error(e.getMessage());
      processRecognitionException(e, sensorContext, inputFile);
    } catch (Exception e) {
      checkInterrupted(e);
      processException(e, sensorContext, inputFile);
-      LOG.error("Unable to analyse file: " + inputFile.absolutePath(), e);
+      LOG.error("Unable to analyse file: " + inputFile.toString(), e);


@vilchik-elena same as above

inverno · 2017-11-28T11:11:51Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptSquidSensor.java

    boolean isMinified = new MinificationAssessor().isMinified(file);
    if (isMinified) {
-      LOG.debug("File [" + file.absolutePath() + "] looks like a minified file and will not be analyzed");
+      LOG.debug("File [" + file.toString() + "] looks like a minified file and will not be analyzed");


@vilchik-elena same

inverno · 2017-11-28T11:33:58Z

...ipt-frontend/src/main/java/org/sonar/plugins/javascript/api/visitors/TreeVisitorContext.java

@@ -35,17 +35,10 @@
  /**
   * @return the current file
   */
-  JavaScriptFile getJavaScriptFile();
+  InputFile getJavaScriptFile();


@vilchik-elena JavaScriptFile -> InputFile ?

inverno · 2017-11-28T11:37:12Z

...-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptSquidSensorTest.java

    createSensor().analyseFiles(context, ImmutableList.of((TreeVisitor) check), ImmutableList.of(file), executor, progressReport);
    assertThat(context.allAnalysisErrors()).hasSize(1);

-    assertThat(logTester.logs()).contains("Unable to analyse file: " + file.absolutePath());
+    assertThat(logTester.logs()).contains("Unable to analyse file: " + file.toString());


@vilchik-elena if you follow my suggestion on file.uri() you'll have to update here as well

SonarTech · 2017-11-28T13:36:11Z

javascript-frontend/src/main/java/org/sonar/javascript/visitors/JavaScriptFileImpl.java

+
+  @Override
+  public String relativePath() {
+    return inputFile.relativePath();


Remove this use of "relativePath"; it is deprecated.

SonarTech · 2017-11-28T13:36:12Z

SonarQube analysis reported 7 issues

6 minor
1 info

Watch the comments in this conversation to review them.

4 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

MetricsVisitor.java#L128: Remove this use of "COMPLEXITY_IN_CLASSES"; it is deprecated.
MetricsVisitor.java#L129: Remove this use of "COMPLEXITY_IN_FUNCTIONS"; it is deprecated.
MetricsVisitor.java#L138: Remove this use of "FUNCTION_COMPLEXITY_DISTRIBUTION"; it is deprecated.
MetricsVisitor.java#L146: Remove this use of "FILE_COMPLEXITY_DISTRIBUTION"; it is deprecated.

SonarTech · 2017-11-28T13:36:12Z

javascript-frontend/src/main/java/org/sonar/plugins/javascript/api/visitors/JavaScriptFile.java

+  /**
+   * @deprecated since 4.0, use {@link JavaScriptFile#fileName()} or {@link JavaScriptFile#uri()}
+   */
+  @Deprecated
  String relativePath();


Do not forget to remove this deprecated code someday.

inverno · 2017-11-28T17:27:17Z

javascript-checks/src/main/java/org/sonar/javascript/checks/FileHeaderCheck.java

@@ -68,7 +68,7 @@ private void checkPlainText() {
    if (expectedLines == null) {
      expectedLines = headerFormat.split("(?:\r)?\n|\r");
    }
-    InputFile file = getContext().getJavaScriptFile();
+    JavaScriptFile file = getContext().getJavaScriptFile();


@vilchik-elena why taking all this pain to wrap InputFile inside JavaScriptFile? The only advantage I would see is to isolate the checks from the plugin-api, but we still have it listed as a dependency in the pom (I presume for good reason) and even then I'm not sure it's worth it. Let me know if I'm missing something

ghost assigned vilchik-elena Nov 24, 2017

ghost added the in progress label Nov 24, 2017

vilchik-elena changed the title ~~bump sonar-plugin-api to 6.7~~ support 6.7 LTS Nov 24, 2017

vilchik-elena added 2 commits November 27, 2017 10:42

bump sonar-plugin-api to 6.7

3c9a5b0

wip coverage

32aac6e

vilchik-elena force-pushed the issue-837 branch from 4191fe1 to 32aac6e Compare November 27, 2017 09:43

vilchik-elena added 4 commits November 27, 2017 10:50

clean ITs

dfdde35

fix some quality flaws

c0df6fc

Fix IT for coverage

1453a23

Drop CompatibleInputFile

f6f5e12

vilchik-elena force-pushed the issue-837 branch from 6b17a21 to f6f5e12 Compare November 27, 2017 15:28

SonarTech reviewed Nov 27, 2017

View reviewed changes

vilchik-elena requested a review from inverno November 27, 2017 15:42

inverno reviewed Nov 28, 2017

View reviewed changes

post-review fixes

ae75c69

SonarTech reviewed Nov 28, 2017

View reviewed changes

inverno reviewed Nov 28, 2017

View reviewed changes

vilchik-elena merged commit c3d804e into master Nov 29, 2017

ghost removed the in progress label Nov 29, 2017

vilchik-elena deleted the issue-837 branch November 29, 2017 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support 6.7 LTS #840

support 6.7 LTS #840

vilchik-elena commented Nov 24, 2017

SonarTech Nov 27, 2017

inverno Nov 28, 2017

vilchik-elena Nov 28, 2017

inverno Nov 28, 2017

vilchik-elena Nov 28, 2017

inverno Nov 28, 2017

vilchik-elena Nov 28, 2017

inverno Nov 28, 2017

vilchik-elena Nov 28, 2017

inverno Nov 28, 2017

inverno Nov 28, 2017

inverno Nov 28, 2017

vilchik-elena Nov 28, 2017

inverno Nov 28, 2017

SonarTech Nov 28, 2017

SonarTech commented Nov 28, 2017

SonarTech Nov 28, 2017

inverno Nov 28, 2017

support 6.7 LTS #840

support 6.7 LTS #840

Conversation

vilchik-elena commented Nov 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SonarTech commented Nov 28, 2017

4 extra issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment