From 3d0f9b58b7840d34e183e8ec8a6b5d5edcdac3b0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Thu, 17 Aug 2023 10:34:57 +0200 Subject: [PATCH] SONAR-20021 Fix the code smells --- .../issue/TrackerRawInputFactory.java | 20 ++++++++-------- .../org/sonar/db/issue/IndexedIssueDto.java | 10 ++++++-- .../java/org/sonar/db/issue/IssueTesting.java | 2 +- .../sonar/server/issue/IssueFieldsSetter.java | 1 - .../sonar/server/issue/index/IssueDoc.java | 12 +++++++--- .../index/IssueIteratorForSingleChunk.java | 4 ++-- .../sonar/server/issue/SearchRequestTest.java | 14 ++++++++--- .../rule/registration/NewRuleCreator.java | 3 ++- .../rule/registration/RulesKeyVerifier.java | 1 - .../rule/registration/StartupRuleUpdater.java | 23 ++++++++++--------- .../org/sonar/core/issue/DefaultIssue.java | 2 -- .../sensor/issue/internal/DefaultIssue.java | 2 -- 12 files changed, 56 insertions(+), 38 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java index 5da64cf469c..7b8696d3fbd 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java @@ -201,6 +201,16 @@ public class TrackerRawInputFactory { return issue; } + private Map convertImpacts(RuleKey ruleKey, List overridenImpactsList) { + if (overridenImpactsList.isEmpty()) { + return Collections.emptyMap(); + } + Rule rule = ruleRepository.getByKey(ruleKey); + return overridenImpactsList.stream() + .filter(i -> rule.getDefaultImpacts().containsKey(SoftwareQuality.valueOf(i.getSoftwareQuality()))) + .collect(Collectors.toMap(i -> SoftwareQuality.valueOf(i.getSoftwareQuality()), i -> org.sonar.api.issue.impact.Severity.valueOf(i.getSeverity()))); + } + private DbIssues.Flow.Builder convertLocations(ScannerReport.Flow flow) { DbIssues.Flow.Builder dbFlowBuilder = DbIssues.Flow.newBuilder(); for (ScannerReport.IssueLocation location : flow.getLocationList()) { @@ -213,6 +223,7 @@ public class TrackerRawInputFactory { return dbFlowBuilder; } + private DefaultIssue toExternalIssue(LineHashSequence lineHashSeq, ScannerReport.ExternalIssue reportExternalIssue, Map adHocRuleMap) { DefaultIssue issue = new DefaultIssue(); RuleKey ruleKey = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + reportExternalIssue.getEngineId(), reportExternalIssue.getRuleId()); @@ -362,15 +373,6 @@ public class TrackerRawInputFactory { } - private Map convertImpacts(RuleKey ruleKey, List overridenImpactsList) { - if (overridenImpactsList.isEmpty()) { - return Collections.emptyMap(); - } - Rule rule = ruleRepository.getByKey(ruleKey); - return overridenImpactsList.stream() - .filter(i -> rule.getDefaultImpacts().containsKey(SoftwareQuality.valueOf(i.getSoftwareQuality()))) - .collect(Collectors.toMap(i -> SoftwareQuality.valueOf(i.getSoftwareQuality()), i -> org.sonar.api.issue.impact.Severity.valueOf(i.getSeverity()))); - } private static DbIssues.MessageFormattings convertMessageFormattings(List msgFormattings) { DbIssues.MessageFormattings.Builder builder = DbIssues.MessageFormattings.newBuilder(); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IndexedIssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IndexedIssueDto.java index dc502006d56..b4f84bfa9f3 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IndexedIssueDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IndexedIssueDto.java @@ -242,12 +242,18 @@ public final class IndexedIssueDto { return this; } - @Deprecated + /** + * @deprecated since 10.2 + */ + @Deprecated(since = "10.2") public Integer getIssueType() { return issueType; } - @Deprecated + /** + * @deprecated since 10.2 + */ + @Deprecated(since = "10.2") public IndexedIssueDto setIssueType(Integer issueType) { this.issueType = issueType; return this; diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueTesting.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueTesting.java index 86ff8712eb5..31362af16c0 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueTesting.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueTesting.java @@ -69,7 +69,7 @@ public class IssueTesting { .setStatus(Issue.STATUS_OPEN) .setResolution(null) .setSeverity(Severity.ALL.get(nextInt(Severity.ALL.size()))) - //TODO map to correct impact + //TODO map to correct impact. Will be fixed with persistence of impacts on issues .addImpact(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(SoftwareQuality.MAINTAINABILITY).setSeverity(org.sonar.api.issue.impact.Severity.HIGH)) .setEffort((long) RandomUtils.nextInt(10)) .setAssigneeUuid("assignee-uuid_" + randomAlphabetic(26)) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java index 0aeaa11cadd..d1e640604cf 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java @@ -24,7 +24,6 @@ import java.time.temporal.ChronoUnit; import java.util.Collection; import java.util.Date; import java.util.EnumMap; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Objects; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java index f2e8f5986f4..0b0fd257a1c 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java @@ -141,7 +141,10 @@ public class IssueDoc extends BaseDoc { return getNullableField(IssueIndexDefinition.FIELD_ISSUE_AUTHOR_LOGIN); } - @Deprecated + /** + * @deprecated since 10.2 + */ + @Deprecated(since = "10.2") public RuleType type() { return RuleType.valueOf(getField(IssueIndexDefinition.FIELD_ISSUE_TYPE)); } @@ -203,7 +206,10 @@ public class IssueDoc extends BaseDoc { return this; } - @Deprecated + /** + * @deprecated since 10.2 + */ + @Deprecated(since = "10.2") public IssueDoc setSeverity(@Nullable String s) { setField(IssueIndexDefinition.FIELD_ISSUE_SEVERITY, s); setField(IssueIndexDefinition.FIELD_ISSUE_SEVERITY_VALUE, Severity.ALL.indexOf(s)); @@ -280,7 +286,7 @@ public class IssueDoc extends BaseDoc { return this; } - @Deprecated + @Deprecated(since = "10.2") public IssueDoc setType(RuleType type) { setField(IssueIndexDefinition.FIELD_ISSUE_TYPE, type.toString()); return this; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java index deec19dbba9..d83383d3557 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java @@ -92,8 +92,8 @@ class IssueIteratorForSingleChunk implements IssueIterator { String cleanCodeAttributeCategory = Optional.ofNullable(indexedIssueDto.getCleanCodeAttribute()) .map(CleanCodeAttribute::valueOf) .map(cleanCodeAttribute -> cleanCodeAttribute.getAttributeCategory().name()) - .orElse(null); - //TODO:: uncomment once clean code attribute is set to not-null + .orElse(null); + //TODO SONAR-20073 uncomment once clean code attribute is set to not-null //.orElseThrow(() -> new IllegalStateException("Clean Code Attribute is missing for issue " + key)); doc.setCleanCodeAttributeCategory(cleanCodeAttributeCategory); doc.setStatus(indexedIssueDto.getStatus()); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/SearchRequestTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/SearchRequestTest.java index 63812baa036..8a536159026 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/SearchRequestTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/SearchRequestTest.java @@ -79,12 +79,20 @@ public class SearchRequestTest { assertThat(underTest.getSort()).isEqualTo("CREATION_DATE"); assertThat(underTest.getAsc()).isTrue(); assertThat(underTest.getInNewCodePeriod()).isTrue(); - assertThat(underTest.getOwaspTop10For2021()).containsExactly("a2", "a3"); - assertThat(underTest.getOwaspAsvs40()).containsExactly("1.1.1", "4.2.2"); - assertThat(underTest.getOwaspAsvsLevel()).isEqualTo(2); + assertOwasp(underTest); assertThat(underTest.getPciDss32()).containsExactly("1", "4"); assertThat(underTest.getPciDss40()).containsExactly("3", "5"); assertThat(underTest.getCodeVariants()).containsExactly("variant1", "variant2"); + assertCleanCodeInformation(underTest); + } + + private static void assertOwasp(SearchRequest underTest) { + assertThat(underTest.getOwaspTop10For2021()).containsExactly("a2", "a3"); + assertThat(underTest.getOwaspAsvs40()).containsExactly("1.1.1", "4.2.2"); + assertThat(underTest.getOwaspAsvsLevel()).isEqualTo(2); + } + + private static void assertCleanCodeInformation(SearchRequest underTest) { assertThat(underTest.getCleanCodeAttributesCategories()).containsExactly("ADAPTABLE"); assertThat(underTest.getImpactSeverities()).containsExactly("HIGH", "LOW"); assertThat(underTest.getImpactSoftwareQualities()).containsExactly("RELIABILITY", "SECURITY"); diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java index 5160840ba1f..ac4fe0adf57 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/NewRuleCreator.java @@ -65,6 +65,7 @@ public class NewRuleCreator { } RuleDto createRuleWithSimpleFields(RulesDefinition.Rule ruleDef, String uuid, long now) { + CleanCodeAttribute cleanCodeAttribute = ruleDef.cleanCodeAttribute(); RuleDto ruleDto = new RuleDto() .setUuid(uuid) .setRuleKey(RuleKey.of(ruleDef.repository().key(), ruleDef.key())) @@ -84,7 +85,7 @@ public class NewRuleCreator { .setIsAdHoc(false) .setCreatedAt(now) .setUpdatedAt(now) - .setCleanCodeAttribute(ruleDef.cleanCodeAttribute() != null ? ruleDef.cleanCodeAttribute() : CleanCodeAttribute.defaultCleanCodeAttribute()) + .setCleanCodeAttribute(cleanCodeAttribute != null ? cleanCodeAttribute : CleanCodeAttribute.defaultCleanCodeAttribute()) .setEducationPrinciples(ruleDef.educationPrincipleKeys()); if (isNotEmpty(ruleDef.htmlDescription())) { diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesKeyVerifier.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesKeyVerifier.java index 767703fbdb9..5f5d07ab53a 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesKeyVerifier.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesKeyVerifier.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import org.sonar.api.rule.RuleKey; diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java index 6b13dc3b76d..d6332e11e90 100644 --- a/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java +++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/StartupRuleUpdater.java @@ -167,8 +167,9 @@ public class StartupRuleUpdater { private static boolean mergeCleanCodeAttribute(RulesDefinition.Rule def, RuleDto dto) { boolean changed = false; - if (!Objects.equals(dto.getCleanCodeAttribute(), def.cleanCodeAttribute()) && (def.cleanCodeAttribute() != null)) { - dto.setCleanCodeAttribute(def.cleanCodeAttribute()); + CleanCodeAttribute defCleanCodeAttribute = def.cleanCodeAttribute(); + if (!Objects.equals(dto.getCleanCodeAttribute(), defCleanCodeAttribute) && (defCleanCodeAttribute != null)) { + dto.setCleanCodeAttribute(defCleanCodeAttribute); changed = true; } // apply non-nullable default @@ -205,7 +206,7 @@ public class StartupRuleUpdater { private static boolean mergeEducationPrinciples(RulesDefinition.Rule ruleDef, RuleDto dto) { boolean changed = false; if (dto.getEducationPrinciples().size() != ruleDef.educationPrincipleKeys().size() || - !dto.getEducationPrinciples().containsAll(ruleDef.educationPrincipleKeys())) { + !dto.getEducationPrinciples().containsAll(ruleDef.educationPrincipleKeys())) { dto.setEducationPrinciples(ruleDef.educationPrincipleKeys()); changed = true; } @@ -219,10 +220,10 @@ public class StartupRuleUpdater { dto.setSystemTags(emptySet()); changed = true; } else if (dto.getSystemTags().size() != ruleDef.tags().size() || - !dto.getSystemTags().containsAll(ruleDef.tags())) { - dto.setSystemTags(ruleDef.tags()); - changed = true; - } + !dto.getSystemTags().containsAll(ruleDef.tags())) { + dto.setSystemTags(ruleDef.tags()); + changed = true; + } return changed; } @@ -233,10 +234,10 @@ public class StartupRuleUpdater { dto.setSecurityStandards(emptySet()); changed = true; } else if (dto.getSecurityStandards().size() != ruleDef.securityStandards().size() || - !dto.getSecurityStandards().containsAll(ruleDef.securityStandards())) { - dto.setSecurityStandards(ruleDef.securityStandards()); - changed = true; - } + !dto.getSecurityStandards().containsAll(ruleDef.securityStandards())) { + dto.setSecurityStandards(ruleDef.securityStandards()); + changed = true; + } return changed; } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index e2a15fd29b6..dfd104b19cb 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -45,8 +45,6 @@ import org.sonar.api.issue.Issue; import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; -import org.sonar.api.rules.CleanCodeAttribute; -import org.sonar.api.rules.CleanCodeAttributeCategory; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.core.issue.tracking.Trackable; diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java index 38c0d61e041..0f835b5ae04 100644 --- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java +++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssue.java @@ -20,9 +20,7 @@ package org.sonar.api.batch.sensor.issue.internal; import java.util.ArrayList; -import java.util.Collections; import java.util.EnumMap; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -- 2.39.5