fix: open/ignored issue filtering [IDE-942] (#265)

The previous filtering logic was sound but confusing to follow, which led to bugs in the predicates (fixed in this commit).
Plus fixed other bugs with open & ignored issue filtering.

Author: rrama

CHANGELOG.md

@@ -6,6 +6,9 @@
 - add scan summary to custom UI, updating scan statuses live
 - added support for DeepCode AI Fixes
 
+### Fixes
+- fixes open & ignored issue filtering toggles
+
 ## [3.0.0]
 ### Changes
 - process api URL from hasAuthenticated message

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/TreeViewerFilter.java

@@ -48,7 +48,7 @@ private boolean isIssueVisible(IssueTreeNode issueNode) {
 	    }
 
 	    for (var filter : this.filters.values()) {
-	        if (!filter.test(issue)) {
+	        if (filter.test(issue)) {
 	            return false;
 	        }
 	    }

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/BaseFilter.java

@@ -23,9 +23,9 @@ public void applyFilter() {
 		boolean booleanPref = this.preferences.getBooleanPref(this.filterName);
 
 		if (booleanPref) {
-			this.filterManager.removeTreeFilter(this.filterName);
+			this.filterManager.removeTreeFilter(this.filterName); // Showing "a thing" removes it from being filtered out.
 		} else {
-			this.filterManager.addTreeFilter(this.filterName, predicate);
+			this.filterManager.addTreeFilter(this.filterName, predicate); // Hiding (unchecking) "a thing" adds a filter to filter it out.
 		}
 	}
-}
+}

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/FixableFilter.java

@@ -8,15 +8,17 @@
 import io.snyk.languageserver.protocolextension.messageObjects.scanResults.Issue;
 
 public class FixableFilter extends BaseFilter {	
-	private static final Predicate<Issue> predicate = issue -> issue.hasFix();
+	private static final Predicate<Issue> predicate = issue -> !issue.hasFix(); // Inverse predicate, see below.
 
 	public FixableFilter(TreeFilterManager tfm) {
 		super(FILTER_SHOW_ONLY_FIXABLE, predicate, tfm);
 	}
 
 	@Override
 	public void applyFilter() {
-		// this is inverse to the other filters, so we need to overwrite the BaseFilter logic
+		// This is inverse to the other filters, so we need to overwrite the BaseFilter logic.
+		// Deselecting show fixable issues should not hide fixable issues and show all issues.
+		// Therefore when deactivated we cannot have a filter added, so the logic is flipped.
 		boolean booleanPref = preferences.getBooleanPref(this.filterName);
 
 		if (booleanPref) {

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/IgnoresIgnoredIssuesFilter.java

@@ -14,4 +14,4 @@ public IgnoresIgnoredIssuesFilter(TreeFilterManager tfm) {
 		super(FILTER_IGNORES_SHOW_IGNORED_ISSUES, predicate, tfm);
 	}
 
-}
+}

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/IgnoresOpenIssuesFilter.java

@@ -1,16 +1,16 @@
 package io.snyk.eclipse.plugin.views.snyktoolview.filters;
 
-import static io.snyk.eclipse.plugin.preferences.Preferences.FILTER_IGNORES_SHOW_IGNORED_ISSUES;
+import static io.snyk.eclipse.plugin.preferences.Preferences.FILTER_IGNORES_SHOW_OPEN_ISSUES;
 
 import java.util.function.Predicate;
 
 import io.snyk.eclipse.plugin.views.snyktoolview.TreeFilterManager;
 import io.snyk.languageserver.protocolextension.messageObjects.scanResults.Issue;
 
-public class IgnoresOpenIssuesFilter extends BaseFilter {	
+public class IgnoresOpenIssuesFilter extends BaseFilter {
 	private static final Predicate<Issue> predicate = issue -> !issue.isIgnored();
 
 	public IgnoresOpenIssuesFilter(TreeFilterManager tfm) {
-		super(FILTER_IGNORES_SHOW_IGNORED_ISSUES, predicate, tfm);
+		super(FILTER_IGNORES_SHOW_OPEN_ISSUES, predicate, tfm);
 	}
-}
+}

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/ProductFilter.java

@@ -22,7 +22,7 @@ public ProductFilter(TreeFilterManager tfm, String filterName) {
 	@SuppressWarnings("rawtypes")
 	private static Predicate getPredicate(String filterName) {
 		String filterableIssueType = preferenceToProductConstants.get(filterName);
-		Predicate<Issue> predicate = issue -> !issue.filterableIssueType().equals(filterableIssueType);
+		Predicate<Issue> predicate = issue -> issue.filterableIssueType().equals(filterableIssueType);
 		return predicate;
 	}
-}
+}

plugin/src/main/java/io/snyk/eclipse/plugin/views/snyktoolview/filters/SeverityFilter.java

@@ -21,7 +21,7 @@ public SeverityFilter(TreeFilterManager tfm, String preferenceKey) {
 	@SuppressWarnings("rawtypes")
 	private static Predicate getPredicate(String preferenceKey) {
 		String severityCondition = preferenceToProductConstants.get(preferenceKey);
-		Predicate<Issue> predicate = issue -> !issue.severity().equals(severityCondition);
+		Predicate<Issue> predicate = issue -> issue.severity().equals(severityCondition);
 		return predicate;
-	}	
-}
+	}
+}

plugin/src/main/java/io/snyk/languageserver/SnykIssueCache.java

@@ -56,8 +56,8 @@ public void clearAll() {
 	 * @param path The file path for which issues should be removed
 	 */
 	public void removeAllIssuesForPath(String path) {
-		codeSecurityIssues.clear();
-		codeQualityIssues.clear();
+		codeSecurityIssues.remove(path);
+		codeQualityIssues.remove(path);
 		ossIssues.remove(path);
 		iacIssues.remove(path);
 	}

tests/src/test/java/io/snyk/eclipse/plugin/views/snyktoolview/TreeViewerFilterTest.java

@@ -43,10 +43,10 @@ void testSelect_IssueTreeNodeWithMatchingFilter() {
         Issue issue = mock(Issue.class);
         when(issueNode.getIssue()).thenReturn(issue);
 
-        Predicate<Issue> predicate = i -> true; // Always matches
+        Predicate<Issue> predicate = i -> true; // Always matches, so filtered out.
         filter.putFilterPredicate("test", predicate);
 
-        assertTrue(filter.select(mockViewer, null, issueNode));
+        assertFalse(filter.select(mockViewer, null, issueNode));
     }
 
     @Test
@@ -55,10 +55,10 @@ void testSelect_IssueTreeNodeWithNonMatchingFilter() {
         Issue issue = mock(Issue.class);
         when(issueNode.getIssue()).thenReturn(issue);
 
-        Predicate<Issue> predicate = i -> false; // Never matches
+        Predicate<Issue> predicate = i -> false; // Never matches, so not filtered out.
         filter.putFilterPredicate("test", predicate);
 
-        assertFalse(filter.select(mockViewer, null, issueNode));
+        assertTrue(filter.select(mockViewer, null, issueNode));
     }
 
     @Test
@@ -85,13 +85,17 @@ void testSelect_FileTreeNodeWithNoVisibleChildren() {
 
     @Test
     void testRemoveFilterPredicate() {
-        Predicate<Issue> predicate = i -> false;
+        IssueTreeNode issueNode = mock(IssueTreeNode.class);
+        Issue issue = mock(Issue.class);
+        when(issueNode.getIssue()).thenReturn(issue);
+
+        Predicate<Issue> predicate = i -> true; // Always matches, so filtered out.
         filter.putFilterPredicate("test", predicate);
 
-        assertTrue(filter.select(mockViewer, null, mock(IssueTreeNode.class)));
+        assertFalse(filter.select(mockViewer, null, issueNode));
 
         filter.removeFilterPredicate("test");
 
-        assertTrue(filter.select(mockViewer, null, mock(IssueTreeNode.class)));
+        assertTrue(filter.select(mockViewer, null, issueNode));
     }
-}
+}