Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancements to support issues and duplications. #1

Closed
wants to merge 5 commits into from

Conversation

ggallen
Copy link

@ggallen ggallen commented Jan 16, 2014

Added code supporting:

  • Cane violations.
  • Reek smells.
  • Roodi problems.
  • Flay code duplication.

Fix some other problems I stumbled across as well.

@ggallen ggallen closed this Jan 23, 2014
@ggallen ggallen reopened this Jan 23, 2014
@theFigler theFigler mentioned this pull request Apr 22, 2014
@ghost
Copy link

ghost commented Aug 7, 2014

Hey there :) Sorry we didn't get to this sooner- A few of us checked out this change and are looking forward to finally merging this! We captured a task in our system "HOSTEXP-933" to track this. We upgraded our system to 3.7.4 (LTS) and did a quick run and ran into errors with metric_fu which we'll have to sort out before we can accept this pull request (always good to see it work before accepting the pull request :)).

What version of metric fu are you using and where are the reports located relative to your project? BTW, we are running like this (not sure if you had other parameters passed?):
metric_fu -r

@ghost
Copy link

ghost commented Aug 7, 2014

Just a heads up, we pulled in a more recent change (hence the conflict this now has). Since you updated that same code, you can resolve the conflict by having git prefer your version and ignoring the change basically

@mrowe
Copy link

mrowe commented Aug 19, 2014

Um, if by

we pulled in a more recent change

you mean my change to make the metric_fu report file relative to the project dir (0e78ba0), then this PR actually reverts that. :( Here is a patch (against this PR) that puts it back:

commit ec7ff7b7693ab45609d46262d5ce4b2f389583d4
Author: Michael Rowe <[email protected]>
Date:   Tue Aug 19 16:48:29 2014 +1000

    Look for the metric_fu report in the project's base dir

diff --git a/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParser.java b/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParser.java
index 58c5914..d04b4bf 100644
--- a/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParser.java
+++ b/src/main/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParser.java
@@ -3,6 +3,7 @@ package com.godaddy.sonar.ruby.metricfu;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -10,12 +11,15 @@ import java.util.regex.Pattern;

 import org.apache.log4j.Logger;
 import org.sonar.api.BatchExtension;
+import org.sonar.api.scan.filesystem.FileQuery;
+import org.sonar.api.scan.filesystem.ModuleFileSystem;
 import org.yaml.snakeyaml.Yaml;

 import com.godaddy.sonar.ruby.metricfu.FlayReason.Match;

 public class MetricfuYamlParser implements BatchExtension {
-   private Logger logger = Logger.getLogger(MetricfuYamlParser.class);
+    private final ModuleFileSystem moduleFileSystem;
+    private Logger logger = Logger.getLogger(MetricfuYamlParser.class);

    private static final String REPORT_FILE = "tmp/metric_fu/report.yml";
    private static Pattern escapePattern = Pattern.compile("\\e\\[\\d+m", Pattern.CASE_INSENSITIVE);
@@ -28,15 +32,16 @@ public class MetricfuYamlParser implements BatchExtension {
    ArrayList<Map<String, Object>> reekFiles = null;
    ArrayList<Map<String, Object>> flayReasons = null;

-   public MetricfuYamlParser() {
-       this(REPORT_FILE);
-   }
+   public MetricfuYamlParser(ModuleFileSystem moduleFileSystem) {
+       this(moduleFileSystem, REPORT_FILE);
+    }

    @SuppressWarnings("unchecked")
-   public MetricfuYamlParser(String filename) {
+   public MetricfuYamlParser(ModuleFileSystem moduleFileSystem, String filename) {
+        this.moduleFileSystem = moduleFileSystem;

        try {
-           FileInputStream input = new FileInputStream(new File(filename));
+            FileInputStream input = new FileInputStream(new File(moduleFileSystem.baseDir(), filename));
            Yaml yaml = new Yaml();

            this.metricfuResult = (Map<String, Object>)yaml.loadAs(input, Map.class);
@@ -240,9 +245,4 @@ public class MetricfuYamlParser implements BatchExtension {
            return 0;
        }
    }
-
-   public static void main(String[] args) {
-       MetricfuYamlParser parser = new MetricfuYamlParser("/home/gallen/work/report.yml");
-       parser.parseFlay();
-   }
 }
diff --git a/src/test/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParserTest.java b/src/test/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParserTest.java
index a487c40..2bb2b2f 100644
--- a/src/test/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParserTest.java
+++ b/src/test/java/com/godaddy/sonar/ruby/metricfu/MetricfuYamlParserTest.java
@@ -1,30 +1,44 @@
 package com.godaddy.sonar.ruby.metricfu;

+import java.io.File;
 import java.io.IOException;
 import java.util.List;

 import junit.framework.TestCase;

+import org.easymock.EasyMock;
+import org.easymock.IMocksControl;
 import org.jfree.util.Log;
 import org.junit.Before;
 import org.junit.Test;
+import org.sonar.api.scan.filesystem.ModuleFileSystem;
+
+import static org.easymock.EasyMock.expect;

 public class MetricfuYamlParserTest extends TestCase
 {
-   private final static String YML_FILE_NAME = "src/test/resources/test-data/results.yml";
-   
+   private final static String YML_FILE_NAME = "resources/test-data/results.yml";
+
+    private IMocksControl mocksControl;
+    private ModuleFileSystem moduleFileSystem;
    private MetricfuYamlParser parser = null;

    @Before
    public void setUp() throws Exception
    {
-       parser = new MetricfuYamlParser(YML_FILE_NAME);
+        mocksControl = EasyMock.createControl();
+        moduleFileSystem = mocksControl.createMock(ModuleFileSystem.class);
    }

    @Test
    public void testParseFunction() throws IOException
    {
-       List<SaikuroComplexity> rubyFunctions = parser.parseSaikuro("lib/some_path/foo_bar.rb");
+        expect(moduleFileSystem.baseDir()).andReturn(new File("src/test"));
+        mocksControl.replay();
+
+        parser = new MetricfuYamlParser(moduleFileSystem, YML_FILE_NAME);
+        List<SaikuroComplexity> rubyFunctions = parser.parseSaikuro("lib/some_path/foo_bar.rb");
+        mocksControl.verify();

        SaikuroComplexity rubyFunction0 = new SaikuroComplexity("lib/some_path/foo_bar.rb", 5, "FooBar#validate_user_name", 4);     
        assertTrue(rubyFunctions.size()==2);

@mrowe
Copy link

mrowe commented Aug 19, 2014

Ok, an inline patch was a terrible idea. ;-)

I've created a new PR that incorporates all this and merges cleanly against master: #4

@bsclifton
Copy link

This pull request is now obsolete because it was pulled in with #4

Going to go ahead and close this out

@bsclifton bsclifton closed this Sep 18, 2015
bsclifton added a commit that referenced this pull request Sep 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants