Repository: sscpac/statick

Issue:
Consider filtering exceptions by file earlier
Currently, both file and regex exceptions are run after the tool plugins are run. I'm thinking it might make more sense to do the file filtering earlier - either as the discovery plugin is running or between the discovery plugin and the tool plugin stages. My reasoning is that if there's a large number of ignored files (such as right now, where I'm telling Statick to ignore the contents of .tox/), we still find and fully process them before ignoring them at the very end, which means there's a lot of wasted time on files we don't care about.

Discussion:

Patch:
diff --git a/statick_tool/discovery_plugin.py b/statick_tool/discovery_plugin.py
index 59099c6..69d1868 100644
--- a/statick_tool/discovery_plugin.py
+++ b/statick_tool/discovery_plugin.py
@@ -18,8 +18,13 @@ class DiscoveryPlugin(IPlugin):
         """Gather arguments for plugin."""
         pass
 
-    def scan(self, package, level):
-        """Scan package to discover files for analysis."""
+    def scan(self, package, level, exceptions=None):
+        """
+        Scan package to discover files for analysis.
+
+        If exceptions is passed, then the plugin should (if practical)
+        use it to filter which files the plugin detects.
+        """
         pass
 
     def set_plugin_context(self, plugin_context):
diff --git a/statick_tool/exceptions.py b/statick_tool/exceptions.py
index 03038e4..41cc5ee 100644
--- a/statick_tool/exceptions.py
+++ b/statick_tool/exceptions.py
@@ -62,6 +62,31 @@ class Exceptions(object):
 
         return exceptions
 
+    def filter_file_exceptions_early(self, package, file_list):
+        """
+        Filter files based on file pattern exceptions list.
+
+        Only filters files which have tools=all, intended for use after the
+        discovery plugins have been run (so that Statick doesn't run the tool
+        plugins against files which will be ignored anyway).
+        """
+        exceptions = self.get_exceptions(package)
+        to_remove = []
+        for filename in file_list:
+            removed = False
+            for exception in exceptions["file"]:
+                if exception["tools"] == 'all':
+                    for pattern in exception["globs"]:
+                        if fnmatch.fnmatch(filename, pattern):
+                            to_remove.append(filename)
+                            removed = True
+                            break
+                    if removed:
+                        break
+        file_list = [filename for filename in file_list if filename not in
+                     to_remove]
+        return file_list
+
     def filter_file_exceptions(self, package, exceptions, issues):
         """Filter issues based on file pattern exceptions list."""
         for tool, tool_issues in list(issues.items()):
diff --git a/statick_tool/plugins/discovery/c_discovery_plugin.py b/statick_tool/plugins/discovery/c_discovery_plugin.py
index 41e801f..ef5605b 100644
--- a/statick_tool/plugins/discovery/c_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/c_discovery_plugin.py
@@ -16,7 +16,7 @@ class CDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "C"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for C files."""
         c_files = []
         c_extensions = ('.c', '.cc', '.cpp', '.cxx', '.h', '.hxx', '.hpp')
@@ -40,5 +40,10 @@ class CDiscoveryPlugin(DiscoveryPlugin):
         c_files = list(OrderedDict.fromkeys(c_files))
 
         print("  {} C/C++ files found.".format(len(c_files)))
+        if exceptions:
+            original_file_count = len(c_files)
+            c_files = exceptions.filter_file_exceptions_early(package, c_files)
+            if original_file_count > len(c_files):
+                print("  After filtering, {} C/C++ files will be scanned.".format(len(c_files)))
 
         package["c_src"] = c_files
diff --git a/statick_tool/plugins/discovery/catkin_discovery_plugin.py b/statick_tool/plugins/discovery/catkin_discovery_plugin.py
index c1ec5d7..45f745b 100644
--- a/statick_tool/plugins/discovery/catkin_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/catkin_discovery_plugin.py
@@ -14,7 +14,7 @@ class CatkinDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "catkin"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for catkin files."""
         cmake_file = os.path.join(package.path, "CMakeLists.txt")
         package_file = os.path.join(package.path, "package.xml")
diff --git a/statick_tool/plugins/discovery/cmake_discovery_plugin.py b/statick_tool/plugins/discovery/cmake_discovery_plugin.py
index 760b9e1..099f7b9 100644
--- a/statick_tool/plugins/discovery/cmake_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/cmake_discovery_plugin.py
@@ -17,7 +17,7 @@ class CMakeDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "cmake"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for CMake files."""
         cmake_file = os.path.join(package.path, "CMakeLists.txt")
 
diff --git a/statick_tool/plugins/discovery/java_discovery_plugin.py b/statick_tool/plugins/discovery/java_discovery_plugin.py
index 61a22c7..d356b61 100644
--- a/statick_tool/plugins/discovery/java_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/java_discovery_plugin.py
@@ -16,7 +16,7 @@ class JavaDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "java"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for java files."""
         java_src_files = []
         java_class_files = []
@@ -34,7 +34,18 @@ class JavaDiscoveryPlugin(DiscoveryPlugin):
         java_class_files = list(OrderedDict.fromkeys(java_class_files))
 
         print("  {} java source files found.".format(len(java_src_files)))
+        if exceptions:
+            original_src_file_count = len(java_src_files)
+            java_src_files = exceptions.filter_file_exceptions_early(package, java_src_files)
+            if original_src_file_count > len(java_src_files):
+                print("  After filtering, {} java source files will be scanned.".format(len(java_src_files)))
+
         print("  {} java class files found.".format(len(java_class_files)))
+        if exceptions:
+            original_class_file_count = len(java_class_files)
+            java_class_files = exceptions.filter_file_exceptions_early(package, java_class_files)
+            if original_class_file_count > len(java_class_files):
+                print("  After filtering, {} java class files will be scanned.".format(len(java_class_files)))
 
         package["java_src"] = java_src_files
         package["java_bin"] = java_class_files
diff --git a/statick_tool/plugins/discovery/maven_discovery_plugin.py b/statick_tool/plugins/discovery/maven_discovery_plugin.py
index b2689e3..f2e5bf0 100644
--- a/statick_tool/plugins/discovery/maven_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/maven_discovery_plugin.py
@@ -16,7 +16,7 @@ class MavenDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "maven"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for maven files."""
         top_poms = []
         all_poms = []
@@ -25,6 +25,10 @@ class MavenDiscoveryPlugin(DiscoveryPlugin):
         for root, _, files in os.walk(package.path):
             for f in fnmatch.filter(files, "pom.xml"):
                 full_path = os.path.join(root, f)
+                # Kind of an ugly hack, but it makes sure long paths don't
+                # mess up our depth tracking
+                if exceptions and not exceptions.filter_file_exceptions_early(package, [full_path]):
+                    continue
                 depth = full_path.count(os.sep)
                 if depth < deepest_pom_level:
                     deepest_pom_level = depth
diff --git a/statick_tool/plugins/discovery/python_discovery_plugin.py b/statick_tool/plugins/discovery/python_discovery_plugin.py
index fc379bd..e71e198 100644
--- a/statick_tool/plugins/discovery/python_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/python_discovery_plugin.py
@@ -17,7 +17,7 @@ class PythonDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "python"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for python files."""
         python_files = []
 
@@ -43,5 +43,10 @@ class PythonDiscoveryPlugin(DiscoveryPlugin):
         python_files = list(OrderedDict.fromkeys(python_files))
 
         print("  {} python files found.".format(len(python_files)))
+        if exceptions:
+            original_file_count = len(python_files)
+            python_files = exceptions.filter_file_exceptions_early(package, python_files)
+            if original_file_count > len(python_files):
+                print("  After filtering, {} python files will be scanned.".format(len(python_files)))
 
         package["python_src"] = python_files
diff --git a/statick_tool/plugins/discovery/xml_discovery_plugin.py b/statick_tool/plugins/discovery/xml_discovery_plugin.py
index b3145b1..d96bc02 100644
--- a/statick_tool/plugins/discovery/xml_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/xml_discovery_plugin.py
@@ -16,7 +16,7 @@ class XMLDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "xml"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for XML files."""
         xml_files = []
         globs = ["*.xml", "*.launch"]
@@ -32,5 +32,10 @@ class XMLDiscoveryPlugin(DiscoveryPlugin):
         xml_files = list(OrderedDict.fromkeys(xml_files))
 
         print("  {} XML files found.".format(len(xml_files)))
+        if exceptions:
+            original_file_count = len(xml_files)
+            xml_files = exceptions.filter_file_exceptions_early(package, xml_files)
+            if original_file_count > len(xml_files):
+                print("  After filtering, {} XML files will be scanned.".format(len(xml_files)))
 
         package["xml"] = xml_files
diff --git a/statick_tool/plugins/discovery/yaml_discovery_plugin.py b/statick_tool/plugins/discovery/yaml_discovery_plugin.py
index b011028..5d3b5f6 100644
--- a/statick_tool/plugins/discovery/yaml_discovery_plugin.py
+++ b/statick_tool/plugins/discovery/yaml_discovery_plugin.py
@@ -16,7 +16,7 @@ class YAMLDiscoveryPlugin(DiscoveryPlugin):
         """Get name of discovery type."""
         return "yaml"
 
-    def scan(self, package, level):
+    def scan(self, package, level, exceptions=None):
         """Scan package looking for YAML files."""
         yaml_files = []
         globs = ["*.yaml"]
@@ -32,5 +32,10 @@ class YAMLDiscoveryPlugin(DiscoveryPlugin):
         yaml_files = list(OrderedDict.fromkeys(yaml_files))
 
         print("  {} YAML files found.".format(len(yaml_files)))
+        if exceptions:
+            original_file_count = len(yaml_files)
+            yaml_files = exceptions.filter_file_exceptions_early(package, yaml_files)
+            if original_file_count > len(yaml_files):
+                print("  After filtering, {} YAML files will be scanned.".format(len(yaml_files)))
 
         package["yaml"] = yaml_files
diff --git a/statick_tool/statick.py b/statick_tool/statick.py
index c9f529b..5c1769a 100644
--- a/statick_tool/statick.py
+++ b/statick_tool/statick.py
@@ -156,7 +156,7 @@ class Statick(object):
             plugin = self.discovery_plugins[plugin_name]
             plugin.set_plugin_context(plugin_context)
             print("Running {} discovery plugin...".format(plugin.get_name()))
-            plugin.scan(package, level)
+            plugin.scan(package, level, self.exceptions)
             print("{} discovery plugin done.".format(plugin.get_name()))
         print("---Discovery---")
 


Test patch:
diff --git a/tests/exceptions/early_exceptions.yaml b/tests/exceptions/early_exceptions.yaml
new file mode 100644
index 0000000..a0df353
--- /dev/null
+++ b/tests/exceptions/early_exceptions.yaml
@@ -0,0 +1,7 @@
+global:
+  exceptions:
+    file:
+      - tools: all
+        globs: ['*unlikelystring*']
+      - tools: something
+        globs: ['*uncommontext*']
diff --git a/tests/exceptions/test_exceptions.py b/tests/exceptions/test_exceptions.py
new file mode 100644
index 0000000..f821a8f
--- /dev/null
+++ b/tests/exceptions/test_exceptions.py
@@ -0,0 +1,86 @@
+"""Unit tests for the Exceptions module."""
+import os
+
+import pytest
+
+from statick_tool.exceptions import Exceptions
+from statick_tool.package import Package
+
+
+def test_exceptions_init_valid():
+    """
+    Test that the Exceptions module initializes correctly.
+
+    Expected result: exceptions.exceptions is initialized.
+    """
+    exceptions = Exceptions(os.path.join(os.path.dirname(__file__),
+                                         'valid_exceptions.yaml'))
+    assert exceptions.exceptions
+
+
+def test_exceptions_init_nonexistent():
+    """
+    Test that the Exceptions module throws an IOError if a bad path is given.
+
+    Expected result: IOError thrown.
+    """
+    with pytest.raises(IOError):
+        Exceptions(os.path.join(os.path.dirname(__file__),
+                                'nonexistent_exceptions.yaml'))
+
+
+def test_filter_file_exceptions_early():
+    """
+    Test that filter_file_exceptions_early excludes files.
+
+    Expected result: Empty files list.
+    """
+    exceptions = Exceptions(os.path.join(os.path.dirname(__file__),
+                                         'early_exceptions.yaml'))
+
+    package = Package('test',  os.path.dirname(__file__))
+    files = [os.path.join(os.path.dirname(__file__),
+                          'unlikelystring')]
+
+    filtered_files = exceptions.filter_file_exceptions_early(package, files)
+
+    assert not filtered_files
+
+
+def test_filter_file_exceptions_early_onlyall():
+    """
+    Test that filter_file_exceptions_early only uses exceptions with tools=all.
+
+    Expected result: No change to the files list
+    """
+    exceptions = Exceptions(os.path.join(os.path.dirname(__file__),
+                                         'early_exceptions.yaml'))
+
+    package = Package('test',  os.path.dirname(__file__))
+    files = [os.path.join(os.path.dirname(__file__),
+                          'uncommontext')]
+
+    filtered_files = exceptions.filter_file_exceptions_early(package, files)
+
+    assert filtered_files == files
+
+
+def test_filter_file_exceptions_early_dupes():
+    """
+    Test that filter_file_exceptions_early excludes duplicated files.
+
+    I have no idea why one might have duplicate files, but might as well test it!
+    Expected result: Empty file list.
+    """
+    exceptions = Exceptions(os.path.join(os.path.dirname(__file__),
+                                         'early_exceptions.yaml'))
+
+    package = Package('test',  os.path.dirname(__file__))
+    files = [os.path.join(os.path.dirname(__file__),
+                          'unlikelystring'),
+             os.path.join(os.path.dirname(__file__),
+                          'unlikelystring')]
+
+    filtered_files = exceptions.filter_file_exceptions_early(package, files)
+
+    assert not filtered_files
diff --git a/tests/exceptions/valid_exceptions.yaml b/tests/exceptions/valid_exceptions.yaml
new file mode 100644
index 0000000..3e3226f
--- /dev/null
+++ b/tests/exceptions/valid_exceptions.yaml
@@ -0,0 +1,5 @@
+global:
+  exceptions:
+    file:
+      - tools: all
+        globs: ['*example*']