Skip to content

Commit efd4142

Browse files
guillaume-dequennesonartech
authored andcommitted
SONARPY-3749 Fix incorrect rule id parsing of Mypy reports (#832)
GitOrigin-RevId: bbe07e85a8130bc46393c0b41f47935c9337c100
1 parent 1cf4466 commit efd4142

6 files changed

Lines changed: 77 additions & 1 deletion

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
src/type_hints_noncompliant.py:11:11: error: Incompatible types in assignment (expression has type "def [T: BaseClass] process_data(*, items: str | Iterable[str] = ..., values: str | Iterable[str] = ..., categories: str | Iterable[str] = ..., dependencies: Iterable[str] = ..., resources: Iterable[str] = ..., metadata: Iterable[str] = ..., identifier: str | None = ..., version: str | None = ..., owner: str | None = ..., owner_email: str | None = ..., maintainer: str | None = ..., maintainer_email: str | None = ..., source_url: str | None = ..., license_type: str | None = ..., summary: str | None = ..., detailed_info: str | None = ..., download_location: str | None = ..., is_verbose: bool = ..., is_dry_run: bool = ..., needs_help: bool = ..., handlers: MutableMapping[str, type[Handler]] = ..., handler_packages: str | list[str] | None = ..., config_name: str | PathLike[str] | None = ..., config_args: list[str] | None = ..., handler_options: MutableMapping[str, Mapping[str, tuple[str, str]]] = ..., modules: list[str] | None = ..., module_dir: Mapping[str, str] | None = ..., submodules: list[str] | None = ..., extensions: list[tuple[str, BuildConfig]] | None = ..., header_files: list[str] | None = ..., binary_modules: Sequence[BinaryModule] | None = ..., extension_package: str | None = ..., search_dirs: list[str] | None = ..., additional_path: Never = ..., executable_files: list[str] | None = ..., static_files: list[tuple[str, Sequence[str]]] | None = ..., access_key: str = ..., handler_obj: MutableMapping[str, Handler] = ..., execution_state: MutableMapping[str, bool] = ..., config_options: Mapping[str, Mapping[str, str]] | None = ..., license_key: Never = ..., show_help: bool = ..., display_name: str | Literal[False] = ..., contact_info: str | Literal[False] = ..., contact_address: str | Literal[False] = ..., data_files: Mapping[str, list[str]] = ..., distribution_files: list[tuple[str, str, str]] = ..., include_data: bool | None = ..., exclude_data: Mapping[str, list[str]] | None = ..., base_dir: str | None = ..., external_deps: list[str] = ..., build_deps: list[str] = ..., content_format: str | None = ..., web_links: Mapping[Any, Any] = ..., extra_features: MutableMapping[Any, Any] = ..., license_info: str | None = ..., license_path: Never = ..., license_paths: Iterable[str] | None = ..., runtime_deps: str | Iterable[str] = ..., optional_deps: Mapping[Any, Any] = ..., result_class: type[T] = ..., **attributes: Any) -> ProcessedData", variable has type "def [T: BaseClass] process_data(*, items: str | Iterable[str] = ..., values: str | Iterable[str] = ..., categories: str | Iterable[str] = ..., dependencies: Iterable[str] = ..., resources: Iterable[str] = ..., metadata: Iterable[str] = ..., identifier: str | None = ..., version: str | None = ..., owner: str | None = ..., owner_email: str | None = ..., maintainer: str | None = ..., maintainer_email: str | None = ..., source_url: str | None = ..., license_type: str | None = ..., summary: str | None = ..., detailed_info: str | None = ..., download_location: str | None = ..., is_verbose: bool = ..., is_dry_run: bool = ..., needs_help: bool = ..., handlers: MutableMapping[str, type[Handler]] = ..., handler_packages: str | list[str] | None = ..., config_name: str | PathLike[str] | None = ..., config_args: list[str] | None = ..., handler_options: MutableMapping[str, Mapping[str, tuple[str, str]]] = ..., modules: list[str] | None = ..., module_dir: Mapping[str, str] | None = ..., submodules: list[str] | None = ..., extensions: list[tuple[str, BuildConfig]] | None = ..., header_files: list[str] | None = ..., binary_modules: Sequence[BinaryModule] | None = ..., extension_package: str | None = ..., search_dirs: list[str] | None = ..., additional_path: Never = ..., executable_files: list[str] | None = ..., static_files: list[tuple[str, Sequence[str]]] | None = ..., access_key: str = ..., handler_obj: MutableMapping[str, Handler] = ..., execution_state: MutableMapping[str, bool] = ..., config_options: Mapping[str, Mapping[str, str]] | None = ..., license_key: Never = ..., show_help: bool = ..., display_name: str | Literal[False] = ..., contact_info: str | Literal[False] = ..., contact_address: str | Literal[False] = ..., data_files: Mapping[str, list[str]] = ..., distribution_files: list[tuple[str, str, str]] = ..., include_data: bool | None = ..., exclude_data: Mapping[str, list[str]] | None = ..., base_dir: str | None = ..., external_deps: list[str] = ..., build_deps: list[str] = ..., content_format: str | None = ..., web_links: Mapping[Any, Any] = ..., extra_features: MutableMapping[Any, Any] = ..., license_info: str | None = ..., license_path: Never = ..., license_paths: Iterable[str] | None = ..., runtime_deps: str | Iterable[str] = ..., optional_deps: Mapping[Any, Any] = ..., result_class: type[T] = ..., **attributes: Any) -> T") [assignment]

its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/MypyReportTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package com.sonar.python.it.plugin;
1818

19+
import com.sonar.orchestrator.build.BuildResult;
1920
import com.sonar.python.it.ConcurrentOrchestratorExtension;
2021
import com.sonar.python.it.TestsUtils;
2122
import java.io.File;
@@ -52,6 +53,34 @@ void import_report() {
5253
assertIssue(issues.get(4), "external_mypy:unknown_mypy_rule", "Unused \"type: ignore\" comment");
5354
}
5455

56+
@Test
57+
void long_message_with_brackets_is_parsed_correctly() {
58+
final String projectKey = "mypy_project_long_message";
59+
ORCHESTRATOR.getServer().provisionProject(projectKey, projectKey);
60+
ORCHESTRATOR.getServer().associateProjectToQualityProfile(projectKey, "py", "no_rule");
61+
62+
BuildResult result = ORCHESTRATOR.executeBuild(
63+
ORCHESTRATOR.createSonarScanner()
64+
.setProjectDir(new File("projects/mypy_project"))
65+
.setProjectKey(projectKey)
66+
.setProjectName(projectKey)
67+
.setProperty("sonar.python.mypy.reportPaths", "mypy_output_long_message.txt"));
68+
69+
assertThat(result.isSuccess()).isTrue();
70+
71+
List<Issues.Issue> importedIssues = issues(projectKey);
72+
assertThat(importedIssues).hasSize(1);
73+
74+
Issues.Issue issue = importedIssues.get(0);
75+
assertThat(issue.getRule()).isEqualTo("external_mypy:assignment");
76+
77+
// Message should be truncated to 500 characters + "..."
78+
assertThat(issue.getMessage()).hasSize(503);
79+
assertThat(issue.getMessage()).startsWith("Incompatible types in assignment");
80+
assertThat(issue.getMessage()).contains("BaseClass");
81+
assertThat(issue.getMessage()).endsWith("...");
82+
}
83+
5584
private static void assertIssue(Issues.Issue issue, String rule, String message) {
5685
assertThat(issue.getComponent()).isEqualTo("mypy_project:src/type_hints_noncompliant.py");
5786
assertThat(issue.getRule()).isEqualTo(rule);

python-commons/src/main/java/org/sonar/plugins/python/mypy/MypySensor.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public class MypySensor extends ExternalIssuesSensor {
4242
public static final String REPORT_PATH_KEY = "sonar.python.mypy.reportPaths";
4343

4444
private static final String FALLBACK_RULE_KEY = "unknown_mypy_rule";
45+
private static final int MAX_RULE_KEY_LENGTH = 200;
46+
private static final int MAX_MESSAGE_LENGTH = 500;
4547

4648
// Pattern -> Location ': ' Severity ':' Message '['Code']'
4749
// Location -> File ':' StartLine (':' StartCol (':' EndLine ':' EndCol))
@@ -50,7 +52,7 @@ public class MypySensor extends ExternalIssuesSensor {
5052
private static final String END_LOCATION = "(?::(?<endLine>\\d+):(?<endCol>\\d+))?";
5153

5254
private static final Pattern PATTERN = Pattern
53-
.compile(String.format("^(?<file>[^:]+):%s%s: (?<severity>\\S+[^:]): (?<message>.*?)(?: \\[(?<code>.*)])?\\s*$", START_LOCATION, END_LOCATION));
55+
.compile(String.format("^(?<file>[^:]+):%s%s: (?<severity>\\S+[^:]): (?<message>.*?)(?: \\[(?<code>[^\\]]+)])?\\s*$", START_LOCATION, END_LOCATION));
5456

5557
@Override
5658
protected void importReport(File reportPath, SensorContext context, Set<String> unresolvedInputFiles) throws IOException {
@@ -98,6 +100,20 @@ private static TextReportReader.Issue extractIssue(Matcher m) {
98100
errorCode = FALLBACK_RULE_KEY;
99101
}
100102

103+
// Skip issues with rule keys that are too long (likely due to parsing errors)
104+
if (errorCode.length() > MAX_RULE_KEY_LENGTH) {
105+
LOG.warn("Skipping mypy issue with rule key longer than {} characters at {}:{}",
106+
MAX_RULE_KEY_LENGTH, filePath, lineNumber);
107+
return null;
108+
}
109+
110+
// Mypy messages can be very long, which can cause problems in the UI.
111+
if (message.length() > MAX_MESSAGE_LENGTH) {
112+
LOG.debug("Truncating mypy message from {} to {} characters at {}:{}",
113+
message.length(), MAX_MESSAGE_LENGTH, filePath, lineNumber);
114+
message = message.substring(0, MAX_MESSAGE_LENGTH) + "...";
115+
}
116+
101117
Integer columnNumber = Optional.ofNullable(m.group("startCol"))
102118
.map(Integer::parseInt)
103119
.map(i -> i - 1)

python-commons/src/test/java/org/sonar/plugins/python/mypy/MypySensorTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,34 @@ void invalid_mypy_file() throws IOException {
193193
assertThat(onlyOneLogElement(logTester.logs(Level.DEBUG))).isEqualTo("Cannot parse the line: this is not a mypy output");
194194
}
195195

196+
@Test
197+
void very_long_rule_message() throws IOException {
198+
List<ExternalIssue> externalIssues = executeSensorImporting("mypy_output_long_message.txt");
199+
assertThat(externalIssues).hasSize(1);
200+
201+
ExternalIssue issue = externalIssues.get(0);
202+
203+
assertThat(issue.ruleId()).isEqualTo("assignment");
204+
assertThat(issue.ruleId().length()).isLessThan(20);
205+
206+
// Message should be truncated to 500 characters + "..."
207+
String message = issue.primaryLocation().message();
208+
assertThat(message)
209+
.hasSize(503)
210+
.startsWith("Incompatible types in assignment")
211+
.contains("BaseClass")
212+
.endsWith("...");
213+
}
214+
215+
@Test
216+
void rule_id_longer_than_limit_is_skipped() throws IOException {
217+
List<ExternalIssue> externalIssues = executeSensorImporting("mypy_output_long_rule_id.txt");
218+
assertThat(externalIssues).isEmpty();
219+
220+
assertThat(onlyOneLogElement(logTester.logs(Level.WARN)))
221+
.contains("Skipping mypy issue with rule key longer than 200 characters");
222+
}
223+
196224
private static void assertIssue(ExternalIssue issue, String key, String message, int startLine, int startColumn, int endLine, int endColumn) {
197225
IssueLocation location = issue.primaryLocation();
198226
assertThat(issue.type()).isEqualTo(RuleType.CODE_SMELL);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
mypy/type_hints_noncompliant.py:11:11: error: Incompatible types in assignment (expression has type "def [T: BaseClass] process_data(*, items: str | Iterable[str] = ..., values: str | Iterable[str] = ..., categories: str | Iterable[str] = ..., dependencies: Iterable[str] = ..., resources: Iterable[str] = ..., metadata: Iterable[str] = ..., identifier: str | None = ..., version: str | None = ..., owner: str | None = ..., owner_email: str | None = ..., maintainer: str | None = ..., maintainer_email: str | None = ..., source_url: str | None = ..., license_type: str | None = ..., summary: str | None = ..., detailed_info: str | None = ..., download_location: str | None = ..., is_verbose: bool = ..., is_dry_run: bool = ..., needs_help: bool = ..., handlers: MutableMapping[str, type[Handler]] = ..., handler_packages: str | list[str] | None = ..., config_name: str | PathLike[str] | None = ..., config_args: list[str] | None = ..., handler_options: MutableMapping[str, Mapping[str, tuple[str, str]]] = ..., modules: list[str] | None = ..., module_dir: Mapping[str, str] | None = ..., submodules: list[str] | None = ..., extensions: list[tuple[str, BuildConfig]] | None = ..., header_files: list[str] | None = ..., binary_modules: Sequence[BinaryModule] | None = ..., extension_package: str | None = ..., search_dirs: list[str] | None = ..., additional_path: Never = ..., executable_files: list[str] | None = ..., static_files: list[tuple[str, Sequence[str]]] | None = ..., access_key: str = ..., handler_obj: MutableMapping[str, Handler] = ..., execution_state: MutableMapping[str, bool] = ..., config_options: Mapping[str, Mapping[str, str]] | None = ..., license_key: Never = ..., show_help: bool = ..., display_name: str | Literal[False] = ..., contact_info: str | Literal[False] = ..., contact_address: str | Literal[False] = ..., data_files: Mapping[str, list[str]] = ..., distribution_files: list[tuple[str, str, str]] = ..., include_data: bool | None = ..., exclude_data: Mapping[str, list[str]] | None = ..., base_dir: str | None = ..., external_deps: list[str] = ..., build_deps: list[str] = ..., content_format: str | None = ..., web_links: Mapping[Any, Any] = ..., extra_features: MutableMapping[Any, Any] = ..., license_info: str | None = ..., license_path: Never = ..., license_paths: Iterable[str] | None = ..., runtime_deps: str | Iterable[str] = ..., optional_deps: Mapping[Any, Any] = ..., result_class: type[T] = ..., **attributes: Any) -> ProcessedData", variable has type "def [T: BaseClass] process_data(*, items: str | Iterable[str] = ..., values: str | Iterable[str] = ..., categories: str | Iterable[str] = ..., dependencies: Iterable[str] = ..., resources: Iterable[str] = ..., metadata: Iterable[str] = ..., identifier: str | None = ..., version: str | None = ..., owner: str | None = ..., owner_email: str | None = ..., maintainer: str | None = ..., maintainer_email: str | None = ..., source_url: str | None = ..., license_type: str | None = ..., summary: str | None = ..., detailed_info: str | None = ..., download_location: str | None = ..., is_verbose: bool = ..., is_dry_run: bool = ..., needs_help: bool = ..., handlers: MutableMapping[str, type[Handler]] = ..., handler_packages: str | list[str] | None = ..., config_name: str | PathLike[str] | None = ..., config_args: list[str] | None = ..., handler_options: MutableMapping[str, Mapping[str, tuple[str, str]]] = ..., modules: list[str] | None = ..., module_dir: Mapping[str, str] | None = ..., submodules: list[str] | None = ..., extensions: list[tuple[str, BuildConfig]] | None = ..., header_files: list[str] | None = ..., binary_modules: Sequence[BinaryModule] | None = ..., extension_package: str | None = ..., search_dirs: list[str] | None = ..., additional_path: Never = ..., executable_files: list[str] | None = ..., static_files: list[tuple[str, Sequence[str]]] | None = ..., access_key: str = ..., handler_obj: MutableMapping[str, Handler] = ..., execution_state: MutableMapping[str, bool] = ..., config_options: Mapping[str, Mapping[str, str]] | None = ..., license_key: Never = ..., show_help: bool = ..., display_name: str | Literal[False] = ..., contact_info: str | Literal[False] = ..., contact_address: str | Literal[False] = ..., data_files: Mapping[str, list[str]] = ..., distribution_files: list[tuple[str, str, str]] = ..., include_data: bool | None = ..., exclude_data: Mapping[str, list[str]] | None = ..., base_dir: str | None = ..., external_deps: list[str] = ..., build_deps: list[str] = ..., content_format: str | None = ..., web_links: Mapping[Any, Any] = ..., extra_features: MutableMapping[Any, Any] = ..., license_info: str | None = ..., license_path: Never = ..., license_paths: Iterable[str] | None = ..., runtime_deps: str | Iterable[str] = ..., optional_deps: Mapping[Any, Any] = ..., result_class: type[T] = ..., **attributes: Any) -> T") [assignment]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
mypy/type_hints_noncompliant.py:11:11: error: Incompatible types in assignment [this-is-a-genuinely-very-long-mypy-error-code-that-exceeds-the-maximum-allowed-rule-key-length-of-two-hundred-characters-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]

0 commit comments

Comments
 (0)