Skip to content

Commit

Permalink
SONARPY-1299 Ensure rules unit tests can run on IPython files (#1408)
Browse files Browse the repository at this point in the history
maksim-grebeniuk-sonarsource authored Mar 10, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent a3c4234 commit 50b9bb9
Showing 8 changed files with 157 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -57,6 +57,37 @@ public void quickFixTest() {
PythonQuickFixVerifier.verifyQuickFixMessages(check, before, expectedMessage);
}

@Test
public void quickFixIPythonTest() {
var check = new CaughtExceptionsCheck();
var before = "#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER\n" +
"class CustomException:\n" +
" ...\n" +
"\n" +
"#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER\n" +
"def foo():\n" +
" try:\n" +
" a = %time bar()\n" +
" except CustomException:\n" +
" ...";

var after = "#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER\n" +
"class CustomException(Exception):\n" +
" ...\n" +
"\n" +
"#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER\n" +
"def foo():\n" +
" try:\n" +
" a = %time bar()\n" +
" except CustomException:\n" +
" ...";

var expectedMessage = String.format(CaughtExceptionsCheck.QUICK_FIX_MESSAGE_FORMAT, "CustomException");

PythonQuickFixVerifier.verifyIPython(check, before, after);
PythonQuickFixVerifier.verifyIPythonQuickFixMessages(check, before, expectedMessage);
}

@Test
public void exceptionWithEmptyParenthesisQuickFixTest() {
var check = new CaughtExceptionsCheck();
Original file line number Diff line number Diff line change
@@ -35,6 +35,11 @@ public void test() {
PythonCheckVerifier.verify("src/test/resources/checks/deadStore.py", check);
}

@Test
public void iPythonTest() {
PythonCheckVerifier.verify("src/test/resources/checks/deadStore.ipynb", check);
}

@Test
public void assignment_on_single_line() {
String codeWithIssue = code(
Original file line number Diff line number Diff line change
@@ -19,27 +19,27 @@
*/
package org.sonar.python.checks.quickfix;

import com.sonar.sslr.api.AstNode;
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.sonar.plugins.python.api.PythonCheck;
import org.sonar.plugins.python.api.PythonCheck.PreciseIssue;
import org.sonar.plugins.python.api.PythonFile;
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
import org.sonar.plugins.python.api.PythonVisitorContext;
import org.sonar.plugins.python.api.tree.FileInput;
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
import org.sonar.plugins.python.api.quickfix.PythonTextEdit;
import org.sonar.python.SubscriptionVisitor;
import org.sonar.python.caching.CacheContextImpl;
import org.sonar.python.parser.PythonParser;
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
import org.sonar.plugins.python.api.quickfix.PythonTextEdit;
import org.sonar.python.semantic.ProjectLevelSymbolTable;
import org.sonar.python.tree.IPythonTreeMaker;
import org.sonar.python.tree.PythonTreeMaker;

import static org.assertj.core.api.Assertions.assertThat;
@@ -49,8 +49,32 @@ private PythonQuickFixVerifier() {
}

public static void verify(PythonCheck check, String codeWithIssue, String... codesFixed) {
verify(PythonQuickFixVerifier::createPythonVisitorContext, check, codeWithIssue, codesFixed);
}

public static void verifyNoQuickFixes(PythonCheck check, String codeWithIssue) {
verifyNoQuickFixes(PythonQuickFixVerifier::createPythonVisitorContext, check, codeWithIssue);
}

public static void verifyQuickFixMessages(PythonCheck check, String codeWithIssue, String... expectedMessages) {
verifyQuickFixMessages(PythonQuickFixVerifier::createPythonVisitorContext, check, codeWithIssue, expectedMessages);
}

public static void verifyIPython(PythonCheck check, String codeWithIssue, String... codesFixed) {
verify(PythonQuickFixVerifier::createIPythonVisitorContext, check, codeWithIssue, codesFixed);
}

public static void verifyIPythonNoQuickFixes(PythonCheck check, String codeWithIssue) {
verifyNoQuickFixes(PythonQuickFixVerifier::createIPythonVisitorContext, check, codeWithIssue);
}

public static void verifyIPythonQuickFixMessages(PythonCheck check, String codeWithIssue, String... expectedMessages) {
verifyQuickFixMessages(PythonQuickFixVerifier::createIPythonVisitorContext, check, codeWithIssue, expectedMessages);
}

public static void verify(Function<String, PythonVisitorContext> createVisitorContext, PythonCheck check, String codeWithIssue, String... codesFixed) {
List<PythonCheck.PreciseIssue> issues = PythonQuickFixVerifier
.getIssuesWithQuickFix(check, codeWithIssue);
.getIssuesWithQuickFix(createVisitorContext, check, codeWithIssue);

assertThat(issues)
.as("Number of issues")
@@ -73,9 +97,9 @@ public static void verify(PythonCheck check, String codeWithIssue, String... cod
.isEqualTo(Arrays.asList(codesFixed));
}

public static void verifyNoQuickFixes(PythonCheck check, String codeWithIssue) {
public static void verifyNoQuickFixes(Function<String, PythonVisitorContext> createVisitorContext, PythonCheck check, String codeWithIssue) {
List<PythonCheck.PreciseIssue> issues = PythonQuickFixVerifier
.getIssuesWithQuickFix(check, codeWithIssue);
.getIssuesWithQuickFix(createVisitorContext, check, codeWithIssue);

assertThat(issues)
.as("Number of issues")
@@ -89,9 +113,12 @@ public static void verifyNoQuickFixes(PythonCheck check, String codeWithIssue) {
.isEmpty();
}

public static void verifyQuickFixMessages(PythonCheck check, String codeWithIssue, String... expectedMessages) {
public static void verifyQuickFixMessages(Function<String, PythonVisitorContext> createVisitorContext,
PythonCheck check,
String codeWithIssue,
String... expectedMessages) {
Stream<String> descriptions = PythonQuickFixVerifier
.getIssuesWithQuickFix(check, codeWithIssue)
.getIssuesWithQuickFix(createVisitorContext, check, codeWithIssue)
.stream()
.flatMap(issue -> issue.quickFixes().stream())
.map(PythonQuickFix::getDescription);
@@ -107,17 +134,27 @@ private static List<PreciseIssue> scanFileForIssues(PythonCheck check, PythonVis
return context.getIssues();
}

private static List<PreciseIssue> getIssuesWithQuickFix(PythonCheck check, String codeWithIssue) {
PythonParser parser = PythonParser.create();
PythonQuickFixFile pythonFile = new PythonQuickFixFile(codeWithIssue);
AstNode astNode = parser.parse(pythonFile.content());
FileInput parse = new PythonTreeMaker().fileInput(astNode);
private static List<PreciseIssue> getIssuesWithQuickFix(Function<String, PythonVisitorContext> createVisitorContext, PythonCheck check, String codeWithIssue) {
var visitorContext = createVisitorContext.apply(codeWithIssue);
return scanFileForIssues(check, visitorContext);
}

private static PythonVisitorContext createPythonVisitorContext(String code) {
return createVisitorContext(PythonParser.create(), new PythonTreeMaker(), code);
}

private static PythonVisitorContext createIPythonVisitorContext(String code) {
return createVisitorContext(PythonParser.createIPythonParser(), new IPythonTreeMaker(), code);
}

PythonVisitorContext visitorContext = new PythonVisitorContext(parse,
private static PythonVisitorContext createVisitorContext(PythonParser parser, PythonTreeMaker treeMaker, String code) {
var pythonFile = new PythonQuickFixFile(code);
var astNode = parser.parse(pythonFile.content());
var fileInput = treeMaker.fileInput(astNode);

return new PythonVisitorContext(fileInput,
pythonFile, null, "",
ProjectLevelSymbolTable.empty(), CacheContextImpl.dummyCache());

return scanFileForIssues(check, visitorContext);
}

private static String applyQuickFix(String codeWithIssue, PythonQuickFix quickFix) {
Original file line number Diff line number Diff line change
@@ -74,6 +74,12 @@ public void test_verify() {
PythonQuickFixVerifier.verify(new SimpleCheck(), "a=10", "a!=10");
}

@Test
public void verifyIPython() {
PythonQuickFixVerifier.verifyIPython(new SimpleCheck(), "a=10\n?a", "a!=10\n?a");
PythonQuickFixVerifier.verifyIPythonQuickFixMessages(new SimpleCheck(), "a=10\n?a", "Add '!' here.");
}

@Test
public void test_multiple_lines() {
PythonQuickFixVerifier.verify(new SimpleCheck(), "b \na=10", "b \na!=10");
5 changes: 5 additions & 0 deletions python-checks/src/test/resources/checks/deadStore.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
def fp_when_using_dynamic_object_information():
x = 42 # Noncompliant
x??
x = 24
print(x)
Original file line number Diff line number Diff line change
@@ -19,7 +19,6 @@
*/
package org.sonar.python;

import com.sonar.sslr.api.AstNode;
import java.io.File;
import java.io.IOException;
import java.net.URI;
@@ -35,6 +34,7 @@
import org.sonar.python.caching.CacheContextImpl;
import org.sonar.python.parser.PythonParser;
import org.sonar.python.semantic.ProjectLevelSymbolTable;
import org.sonar.python.tree.IPythonTreeMaker;
import org.sonar.python.tree.PythonTreeMaker;

import static org.sonar.python.semantic.SymbolUtils.pythonPackageName;
@@ -62,25 +62,33 @@ public static PythonVisitorContext createContext(File file, @Nullable File worki

public static PythonVisitorContext createContext(File file, @Nullable File workingDirectory, String packageName,
ProjectLevelSymbolTable projectLevelSymbolTable, CacheContext cacheContext) {
PythonParser parser = PythonParser.create();
TestPythonFile pythonFile = new TestPythonFile(file);
AstNode astNode = parser.parse(pythonFile.content());
FileInput rootTree = new PythonTreeMaker().fileInput(astNode);
FileInput rootTree = parseFile(pythonFile);
return new PythonVisitorContext(rootTree, pythonFile, workingDirectory, packageName, projectLevelSymbolTable, cacheContext);
}

public static ProjectLevelSymbolTable globalSymbols(List<File> files, File baseDir) {
ProjectLevelSymbolTable projectLevelSymbolTable = new ProjectLevelSymbolTable();
for (File file : files) {
TestPythonFile pythonFile = new TestPythonFile(file);
AstNode astNode = PythonParser.create().parse(pythonFile.content());
FileInput astRoot = new PythonTreeMaker().fileInput(astNode);
var pythonFile = new TestPythonFile(file);
if (pythonFile.isIPython()) {
continue;
}
var astRoot = parseFile(pythonFile);
String packageName = pythonPackageName(file, baseDir.getAbsolutePath());
projectLevelSymbolTable.addModule(astRoot, packageName, pythonFile);
}
return projectLevelSymbolTable;
}

private static FileInput parseFile(TestPythonFile file) {
var parser = file.isIPython() ? PythonParser.createIPythonParser() : PythonParser.create();
var treeMaker = file.isIPython() ? new IPythonTreeMaker() : new PythonTreeMaker();

var astNode = parser.parse(file.content());
return treeMaker.fileInput(astNode);
}

private static class TestPythonFile implements PythonFile {

private final File file;
@@ -113,6 +121,10 @@ public String key() {
return file.getPath();
}

public boolean isIPython() {
return fileName().endsWith(".ipynb");
}

}

}
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Collections;
import java.util.List;
import org.junit.Test;
import org.sonar.plugins.python.api.PythonVisitorContext;
import org.sonar.plugins.python.api.symbols.Symbol;
@@ -33,21 +33,41 @@
public class TestPythonVisitorRunnerTest {

@Test(expected = IllegalStateException.class)
public void unknown_file() {
public void unknownFile() {
TestPythonVisitorRunner.scanFile(new File("xxx"), visitorContext -> {});
}

@Test
public void file_uri() throws IOException {
File tmpFile = Files.createTempFile("foo", "py").toFile();
public void fileUri() throws IOException {
File tmpFile = Files.createTempFile("foo", ".py").toFile();
PythonVisitorContext context = TestPythonVisitorRunner.createContext(tmpFile);
assertThat(context.pythonFile().uri()).isEqualTo(tmpFile.toURI());
}

@Test
public void global_symbols() {
public void fileUriIPython() throws IOException {
File tmpFile = Files.createTempFile("foo", ".ipynb").toFile();
PythonVisitorContext context = TestPythonVisitorRunner.createContext(tmpFile);
assertThat(context.pythonFile().uri()).isEqualTo(tmpFile.toURI());
}

@Test
public void globalSymbols() {
File baseDir = new File("src/test/resources").getAbsoluteFile();
List<File> files = List.of(new File(baseDir, "file.py"));
ProjectLevelSymbolTable projectLevelSymbolTable = TestPythonVisitorRunner.globalSymbols(files, baseDir);
assertThat(projectLevelSymbolTable.getSymbolsFromModule("file"))
.extracting(Symbol::name)
.containsExactlyInAnyOrder("hello", "A");
}

@Test
public void globalSymbolsIPython() {
File baseDir = new File("src/test/resources").getAbsoluteFile();
ProjectLevelSymbolTable projectLevelSymbolTable = TestPythonVisitorRunner.globalSymbols(Collections.singletonList(new File(baseDir, "file.py")), baseDir);
assertThat(projectLevelSymbolTable.getSymbolsFromModule("file")).extracting(Symbol::name).containsExactlyInAnyOrder("hello", "A");
List<File> files = List.of(new File(baseDir, "file.py"), new File(baseDir, "file.ipynb"));
ProjectLevelSymbolTable projectLevelSymbolTable = TestPythonVisitorRunner.globalSymbols(files, baseDir);
assertThat(projectLevelSymbolTable.getSymbolsFromModule("file"))
.extracting(Symbol::name)
.containsExactlyInAnyOrder("hello", "A");
}
}
10 changes: 10 additions & 0 deletions python-frontend/src/test/resources/file.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
def hello2():
print("Hello world")
return [
]
#SONAR_PYTHON_NOTEBOOK_CELL_DELIMITER
#A comment
class B:
def method(self):
pass

0 comments on commit 50b9bb9

Please sign in to comment.