From eb9a74051dfdf0b96d0bd00fc79cfca1f389e251 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Mon, 28 Aug 2023 14:41:14 +0200 Subject: [PATCH] SONAR-20260 clean code fields are not populated for security hotspot --- .../issue/AdHocRuleCreator.java | 6 ++- .../projectanalysis/issue/NewAdHocRule.java | 41 ++++++++++--------- .../ce/task/projectanalysis/issue/Rule.java | 1 + .../pushevent/PushEventFactory.java | 6 ++- .../issue/NewAdHocRuleTest.java | 22 ++++++++++ .../java/org/sonar/db/issue/IssueDto.java | 1 + .../main/java/org/sonar/db/rule/RuleDto.java | 3 +- .../org/sonar/db/rule/RuleForIndexingDto.java | 1 + .../migration/version/v102/DbVersion102.java | 2 - ...pulateCleanCodeAttributeColumnInRules.java | 6 ++- ...teCleanCodeAttributeColumnInRulesTest.java | 26 +++++++++++- .../rule/registration/NewRuleCreator.java | 17 ++++---- .../rule/registration/StartupRuleUpdater.java | 3 ++ .../rule/registration/NewRuleCreatorTest.java | 17 ++++++-- .../rule/registration/RulesRegistrantIT.java | 2 + .../server/issue/ws/SearchResponseFormat.java | 8 ++-- .../org/sonar/server/rule/ws/RuleMapper.java | 11 +++-- 17 files changed, 123 insertions(+), 50 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java index 773afc8b070..e59b163e80c 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java @@ -26,6 +26,7 @@ import java.util.Optional; import java.util.stream.Collectors; import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; +import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.System2; import org.sonar.core.util.UuidFactory; @@ -89,8 +90,9 @@ public class AdHocRuleCreator { } } - if (!Objects.equals(ruleDtoToUpdate.getCleanCodeAttribute(), adHoc.getCleanCodeAttribute())) { - ruleDtoToUpdate.setCleanCodeAttribute(adHoc.getCleanCodeAttribute()); + CleanCodeAttribute cleanCodeAttribute = adHoc.getCleanCodeAttribute(); + if (!Objects.equals(ruleDtoToUpdate.getCleanCodeAttribute(), cleanCodeAttribute)) { + ruleDtoToUpdate.setCleanCodeAttribute(cleanCodeAttribute); changed = true; } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java index e83059b5acc..4184229c872 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java @@ -51,8 +51,7 @@ public class NewAdHocRule { private final String severity; private final RuleType ruleType; private final boolean hasDetails; - - private final CleanCodeAttribute cleanCodeAttribute; + private CleanCodeAttribute cleanCodeAttribute = null; private final Map defaultImpacts = new EnumMap<>(SoftwareQuality.class); @@ -70,11 +69,29 @@ public class NewAdHocRule { this.name = ruleFromScannerReport.getName(); this.description = trimToNull(ruleFromScannerReport.getDescription()); this.hasDetails = true; - this.cleanCodeAttribute = mapCleanCodeAttribute(trimToNull(ruleFromScannerReport.getCleanCodeAttribute())); this.ruleType = determineType(ruleFromScannerReport); this.severity = determineSeverity(ruleFromScannerReport); - this.defaultImpacts.putAll(determineImpacts(ruleFromScannerReport)); + if (!ScannerReport.IssueType.SECURITY_HOTSPOT.equals(ruleFromScannerReport.getType())) { + this.cleanCodeAttribute = mapCleanCodeAttribute(trimToNull(ruleFromScannerReport.getCleanCodeAttribute())); + this.defaultImpacts.putAll(determineImpacts(ruleFromScannerReport)); + } + } + public NewAdHocRule(ScannerReport.ExternalIssue fromIssue) { + Preconditions.checkArgument(isNotBlank(fromIssue.getEngineId()), "'engine id' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(isNotBlank(fromIssue.getRuleId()), "'rule id' not expected to be null for an ad hoc rule"); + this.key = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + fromIssue.getEngineId(), fromIssue.getRuleId()); + this.engineId = fromIssue.getEngineId(); + this.ruleId = fromIssue.getRuleId(); + this.name = null; + this.description = null; + this.severity = null; + this.ruleType = null; + this.hasDetails = false; + if (!ScannerReport.IssueType.SECURITY_HOTSPOT.equals(fromIssue.getType())) { + this.cleanCodeAttribute = CleanCodeAttribute.defaultCleanCodeAttribute(); + this.defaultImpacts.put(SoftwareQuality.MAINTAINABILITY, Severity.MEDIUM); + } } private Map determineImpacts(ScannerReport.AdHocRule ruleFromScannerReport) { @@ -131,21 +148,6 @@ public class NewAdHocRule { return SoftwareQuality.valueOf(softwareQuality); } - public NewAdHocRule(ScannerReport.ExternalIssue fromIssue) { - Preconditions.checkArgument(isNotBlank(fromIssue.getEngineId()), "'engine id' not expected to be null for an ad hoc rule"); - Preconditions.checkArgument(isNotBlank(fromIssue.getRuleId()), "'rule id' not expected to be null for an ad hoc rule"); - this.key = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + fromIssue.getEngineId(), fromIssue.getRuleId()); - this.engineId = fromIssue.getEngineId(); - this.ruleId = fromIssue.getRuleId(); - this.name = null; - this.description = null; - this.severity = null; - this.ruleType = null; - this.hasDetails = false; - this.cleanCodeAttribute = CleanCodeAttribute.defaultCleanCodeAttribute(); - this.defaultImpacts.put(SoftwareQuality.MAINTAINABILITY, Severity.MEDIUM); - } - public RuleKey getKey() { return key; } @@ -182,6 +184,7 @@ public class NewAdHocRule { return hasDetails; } + @CheckForNull public CleanCodeAttribute getCleanCodeAttribute() { return cleanCodeAttribute; } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java index 94c0e3eee9a..53e41475825 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java @@ -71,5 +71,6 @@ public interface Rule { Map getDefaultImpacts(); + @CheckForNull CleanCodeAttribute cleanCodeAttribute(); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java index 58c5e0ddeea..0714652f311 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java @@ -30,6 +30,7 @@ import org.jetbrains.annotations.NotNull; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; +import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; @@ -146,8 +147,9 @@ public class PushEventFactory { issue.getRuleDescriptionContextKey().ifPresent(event::setRuleDescriptionContextKey); Rule rule = ruleRepository.getByKey(issue.getRuleKey()); - event.setCleanCodeAttribute(rule.cleanCodeAttribute().name()); - event.setCleanCodeAttributeCategory(rule.cleanCodeAttribute().getAttributeCategory().name()); + CleanCodeAttribute cleanCodeAttribute = requireNonNull(rule.cleanCodeAttribute()); + event.setCleanCodeAttribute(cleanCodeAttribute.name()); + event.setCleanCodeAttributeCategory(cleanCodeAttribute.getAttributeCategory().name()); event.setImpacts(computeEffectiveImpacts(rule.getDefaultImpacts(), issue.impacts())); return event; } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java index 63e7e4a2c2d..a3267b4ddbe 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java @@ -119,4 +119,26 @@ public class NewAdHocRuleTest { .hasMessage("'issue type' not expected to be null for an ad hoc rule, or impacts should be provided instead"); } + @Test + public void constructor_whenRuleHotspot_shouldNotPopulateImpactsNorAttribute() { + NewAdHocRule adHocRule1 = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder() + .setEngineId("eslint") + .setType(ScannerReport.IssueType.SECURITY_HOTSPOT) + .setSeverity(Constants.Severity.BLOCKER) + .setRuleId("no-cond-assign").build()); + assertThat(adHocRule1.getDefaultImpacts()).isEmpty(); + assertThat(adHocRule1.getCleanCodeAttribute()).isNull(); + } + + @Test + public void constructor_whenIssueHotspot_shouldNotPopulateImpactsNorAttribute() { + NewAdHocRule adHocRule1 = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder() + .setType(ScannerReport.IssueType.SECURITY_HOTSPOT) + .setSeverity(Constants.Severity.BLOCKER) + .setEngineId("eslint") + .setRuleId("no-cond-assign").build()); + assertThat(adHocRule1.getDefaultImpacts()).isEmpty(); + assertThat(adHocRule1.getCleanCodeAttribute()).isNull(); + } + } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java index 1c0143c1953..e6811c758f4 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java @@ -771,6 +771,7 @@ public final class IssueDto implements Serializable { return this; } + @CheckForNull public CleanCodeAttribute getCleanCodeAttribute() { return cleanCodeAttribute; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java index 76940f89c4b..25e5f4d77ac 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java @@ -431,11 +431,12 @@ public class RuleDto { return this; } + @CheckForNull public CleanCodeAttribute getCleanCodeAttribute() { return cleanCodeAttribute; } - public RuleDto setCleanCodeAttribute(CleanCodeAttribute cleanCodeAttribute) { + public RuleDto setCleanCodeAttribute(@Nullable CleanCodeAttribute cleanCodeAttribute) { this.cleanCodeAttribute = cleanCodeAttribute; return this; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleForIndexingDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleForIndexingDto.java index 1b6e730b315..d23db79bc15 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleForIndexingDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleForIndexingDto.java @@ -216,6 +216,7 @@ public class RuleForIndexingDto { this.type = type; } + @CheckForNull public String getCleanCodeAttributeCategory() { return cleanCodeAttributeCategory; } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/DbVersion102.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/DbVersion102.java index c988d839b0a..2ee6cb2a90d 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/DbVersion102.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/DbVersion102.java @@ -86,8 +86,6 @@ public class DbVersion102 implements DbVersion { .add(10_2_032, "Increase size of 'ce_queue.main_is_last_key' from 55 to 80 characters", IncreaseMainIsLastKeyInCeActivity.class) .add(10_2_033, "Add column 'clean_code_attribute' in 'rules' table", AddCleanCodeAttributeInRules.class) .add(10_2_034, "Populate 'clean_code_attribute' column in 'rules' table", PopulateCleanCodeAttributeColumnInRules.class) - //TODO SONAR-20073 - //.add(10_2_035, "Make 'clean_code_attribute' column not nullable in 'rules' table", MakeCleanCodeAttributeColumnNotNullableInRules.class); .add(10_2_036, "Create 'rules_default_impacts' table", CreateRulesDefaultImpactsTable.class) .add(10_2_037, "Create unique constraint index on 'rules_default_impacts' table", CreateUniqueConstraintOnRulesDefaultImpacts.class) diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRules.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRules.java index 5faa03483f6..2846c02f956 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRules.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRules.java @@ -25,13 +25,15 @@ import org.sonar.db.Database; import org.sonar.server.platform.db.migration.step.DataChange; import org.sonar.server.platform.db.migration.step.MassUpdate; +import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; + public class PopulateCleanCodeAttributeColumnInRules extends DataChange { private static final String SELECT_QUERY = """ SELECT uuid, clean_code_attribute FROM rules - WHERE clean_code_attribute is null - """; + WHERE clean_code_attribute is null and (rule_type <> %1$s or ad_hoc_type <> %1$s) + """.formatted(SECURITY_HOTSPOT.getDbConstant()); private static final String UPDATE_QUERY = """ UPDATE rules diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRulesTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRulesTest.java index cb2dab2531a..b65783990e8 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRulesTest.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v102/PopulateCleanCodeAttributeColumnInRulesTest.java @@ -24,6 +24,7 @@ import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.sonar.api.rules.CleanCodeAttribute; +import org.sonar.api.rules.RuleType; import org.sonar.db.CoreDbTester; import static org.assertj.core.api.Assertions.assertThat; @@ -68,7 +69,24 @@ public class PopulateCleanCodeAttributeColumnInRulesTest { .extracting(stringObjectMap -> stringObjectMap.get("CLEAN_CODE_ATTRIBUTE")).containsExactly(CleanCodeAttribute.FOCUSED.name()); } - private void insertRule(String uuid, @Nullable CleanCodeAttribute cleanCodeAttribute) { + @Test + public void execute_whenRuleIsHotspot_shouldNotUpdate() throws SQLException { + insertRule("1", RuleType.SECURITY_HOTSPOT, null, null); + underTest.execute(); + assertThat(db.select("select UUID, CLEAN_CODE_ATTRIBUTE from rules")) + .extracting(stringObjectMap -> stringObjectMap.get("CLEAN_CODE_ATTRIBUTE")).containsOnlyNulls(); + } + + @Test + public void execute_whenAdhocRuleIsHotspot_shouldNotUpdate() throws SQLException { + insertRule("1", null, RuleType.SECURITY_HOTSPOT, null); + underTest.execute(); + assertThat(db.select("select UUID, CLEAN_CODE_ATTRIBUTE from rules")) + .extracting(stringObjectMap -> stringObjectMap.get("CLEAN_CODE_ATTRIBUTE")).containsOnlyNulls(); + } + + + private void insertRule(String uuid, @Nullable RuleType ruleType, @Nullable RuleType adhocRuleType, @Nullable CleanCodeAttribute cleanCodeAttribute) { db.executeInsert(TABLE_NAME, "UUID", uuid, "PLUGIN_RULE_KEY", "key", @@ -76,7 +94,13 @@ public class PopulateCleanCodeAttributeColumnInRulesTest { "SCOPE", "1", "CLEAN_CODE_ATTRIBUTE", cleanCodeAttribute != null ? cleanCodeAttribute.name() : null, "IS_TEMPLATE", false, + "RULE_TYPE", ruleType != null ? ruleType.getDbConstant() : null, + "AD_HOC_TYPE", adhocRuleType != null ? adhocRuleType.getDbConstant() : null, "IS_AD_HOC", false, "IS_EXTERNAL", false); } + + private void insertRule(String uuid, @Nullable CleanCodeAttribute cleanCodeAttribute) { + insertRule(uuid, RuleType.CODE_SMELL, null, cleanCodeAttribute); + } } 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 ac4fe0adf57..cb5f2e43260 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 @@ -51,12 +51,6 @@ public class NewRuleCreator { RuleDto createNewRule(RulesRegistrationContext context, RulesDefinition.Rule ruleDef, DbSession session) { RuleDto newRule = createRuleWithSimpleFields(ruleDef, uuidFactory.create(), system2.now()); - - newRule.replaceAllDefaultImpacts(ruleDef.defaultImpacts().entrySet() - .stream() - .map(e -> new ImpactDto().setUuid(uuidFactory.create()).setSoftwareQuality(e.getKey()).setSeverity(e.getValue())) - .collect(Collectors.toSet())); - ruleDescriptionSectionsGeneratorResolver.generateFor(ruleDef).forEach(newRule::addRuleDescriptionSectionDto); dbClient.ruleDao().insert(session, newRule); @@ -65,7 +59,6 @@ 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())) @@ -85,9 +78,17 @@ public class NewRuleCreator { .setIsAdHoc(false) .setCreatedAt(now) .setUpdatedAt(now) - .setCleanCodeAttribute(cleanCodeAttribute != null ? cleanCodeAttribute : CleanCodeAttribute.defaultCleanCodeAttribute()) .setEducationPrinciples(ruleDef.educationPrincipleKeys()); + if (!RuleType.SECURITY_HOTSPOT.equals(ruleDef.type())) { + CleanCodeAttribute cleanCodeAttribute = ruleDef.cleanCodeAttribute(); + ruleDto.setCleanCodeAttribute(cleanCodeAttribute != null ? cleanCodeAttribute : CleanCodeAttribute.defaultCleanCodeAttribute()); + ruleDto.replaceAllDefaultImpacts(ruleDef.defaultImpacts().entrySet() + .stream() + .map(e -> new ImpactDto().setUuid(uuidFactory.create()).setSoftwareQuality(e.getKey()).setSeverity(e.getValue())) + .collect(Collectors.toSet())); + } + if (isNotEmpty(ruleDef.htmlDescription())) { ruleDto.setDescriptionFormat(RuleDto.Format.HTML); } else if (isNotEmpty(ruleDef.markdownDescription())) { 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 d6332e11e90..1eb8793b20c 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 @@ -166,6 +166,9 @@ public class StartupRuleUpdater { } private static boolean mergeCleanCodeAttribute(RulesDefinition.Rule def, RuleDto dto) { + if (dto.getEnumType() == RuleType.SECURITY_HOTSPOT) { + return false; + } boolean changed = false; CleanCodeAttribute defCleanCodeAttribute = def.cleanCodeAttribute(); if (!Objects.equals(dto.getCleanCodeAttribute(), defCleanCodeAttribute) && (defCleanCodeAttribute != null)) { diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java index d16a6f9122c..56cb8262da1 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/NewRuleCreatorTest.java @@ -69,13 +69,22 @@ public class NewRuleCreatorTest { @Test public void from_whenRuleDefinitionDoesHaveCleanCodeAttribute_shouldReturnThisAttribute() { - RulesDefinition.Rule ruleDef = getDefaultRule(CleanCodeAttribute.TESTED); + RulesDefinition.Rule ruleDef = getDefaultRule(CleanCodeAttribute.TESTED, RuleType.CODE_SMELL); RuleDto newRuleDto = underTest.createNewRule(context, ruleDef, mock()); assertThat(newRuleDto.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.TESTED); } + @Test + public void createNewRule_whenRuleDefinitionDoesHaveCleanCodeAttributeAndIsSecurityHotspot_shouldReturnNull() { + RulesDefinition.Rule ruleDef = getDefaultRule(CleanCodeAttribute.TESTED, RuleType.SECURITY_HOTSPOT); + + RuleDto newRuleDto = underTest.createNewRule(context, ruleDef, mock()); + assertThat(newRuleDto.getCleanCodeAttribute()).isNull(); + assertThat(newRuleDto.getDefaultImpacts()).isEmpty(); + } + @Test public void createNewRule_whenImpactDefined_shouldReturnThisImpact() { RulesDefinition.Rule ruleDef = getDefaultRule(); @@ -89,14 +98,14 @@ public class NewRuleCreatorTest { .containsOnly(tuple(SoftwareQuality.RELIABILITY, Severity.LOW)); } - private static RulesDefinition.Rule getDefaultRule(@Nullable CleanCodeAttribute attribute) { + private static RulesDefinition.Rule getDefaultRule(@Nullable CleanCodeAttribute attribute, RuleType ruleType) { RulesDefinition.Rule ruleDef = mock(RulesDefinition.Rule.class); RulesDefinition.Repository repository = mock(RulesDefinition.Repository.class); when(ruleDef.repository()).thenReturn(repository); when(ruleDef.key()).thenReturn("key"); when(repository.key()).thenReturn("repoKey"); - when(ruleDef.type()).thenReturn(RuleType.CODE_SMELL); + when(ruleDef.type()).thenReturn(ruleType); when(ruleDef.scope()).thenReturn(RuleScope.TEST); when(ruleDef.cleanCodeAttribute()).thenReturn(attribute); when(ruleDef.severity()).thenReturn(MAJOR); @@ -105,6 +114,6 @@ public class NewRuleCreatorTest { } private static RulesDefinition.Rule getDefaultRule() { - return getDefaultRule(null); + return getDefaultRule(null, RuleType.CODE_SMELL); } } diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java index 9a936b48888..28b43b9d6a4 100644 --- a/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java +++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/rule/registration/RulesRegistrantIT.java @@ -227,6 +227,8 @@ public class RulesRegistrantIT { assertThat(hotspotRule.getUpdatedAt()).isEqualTo(RulesRegistrantIT.DATE1.getTime()); assertThat(hotspotRule.getType()).isEqualTo(RuleType.SECURITY_HOTSPOT.getDbConstant()); assertThat(hotspotRule.getSecurityStandards()).containsExactly("cwe:1", "cwe:123", "cwe:863", "owaspTop10-2021:a1", "owaspTop10-2021:a3"); + assertThat(hotspotRule.getDefaultImpacts()).isEmpty(); + assertThat(hotspotRule.getCleanCodeAttribute()).isNull(); } @Test diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java index 1852d6dcf9c..e164e6a50ac 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java @@ -175,11 +175,9 @@ public class SearchResponseFormat { issueBuilder.setType(Common.RuleType.forNumber(dto.getType())); CleanCodeAttribute cleanCodeAttribute = dto.getCleanCodeAttribute(); - String cleanCodeAttributeString = cleanCodeAttribute != null ? cleanCodeAttribute.name() : null; - String cleanCodeAttributeCategoryString = cleanCodeAttribute != null ? cleanCodeAttribute.getAttributeCategory().name() : null; - if (cleanCodeAttributeString != null) { - issueBuilder.setCleanCodeAttribute(Common.CleanCodeAttribute.valueOf(cleanCodeAttributeString)); - issueBuilder.setCleanCodeAttributeCategory(Common.CleanCodeAttributeCategory.valueOf(cleanCodeAttributeCategoryString)); + if (cleanCodeAttribute != null) { + issueBuilder.setCleanCodeAttribute(Common.CleanCodeAttribute.valueOf(cleanCodeAttribute.name())); + issueBuilder.setCleanCodeAttributeCategory(Common.CleanCodeAttributeCategory.valueOf(cleanCodeAttribute.getAttributeCategory().name())); } issueBuilder.addAllImpacts(dto.getEffectiveImpacts().entrySet() .stream() diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/RuleMapper.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/RuleMapper.java index 6910edaa6b0..427555e2512 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/RuleMapper.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/ws/RuleMapper.java @@ -32,6 +32,7 @@ import javax.annotation.Nullable; import org.sonar.api.resources.Language; import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.debt.internal.DefaultDebtRemediationFunction; @@ -223,9 +224,10 @@ public class RuleMapper { } private static void setCleanCodeAttributes(Rules.Rule.Builder ruleResponse, RuleDto ruleDto, Set fieldsToReturn) { - if (shouldReturnField(fieldsToReturn, FIELD_CLEAN_CODE_ATTRIBUTE) && ruleDto.getType() != RuleType.SECURITY_HOTSPOT.getDbConstant()) { - ruleResponse.setCleanCodeAttribute(Common.CleanCodeAttribute.valueOf(ruleDto.getCleanCodeAttribute().name())); - ruleResponse.setCleanCodeAttributeCategory(Common.CleanCodeAttributeCategory.valueOf(ruleDto.getCleanCodeAttribute().getAttributeCategory().name())); + CleanCodeAttribute cleanCodeAttribute = ruleDto.getCleanCodeAttribute(); + if (shouldReturnField(fieldsToReturn, FIELD_CLEAN_CODE_ATTRIBUTE) && cleanCodeAttribute != null) { + ruleResponse.setCleanCodeAttribute(Common.CleanCodeAttribute.valueOf(cleanCodeAttribute.name())); + ruleResponse.setCleanCodeAttributeCategory(Common.CleanCodeAttributeCategory.valueOf(cleanCodeAttribute.getAttributeCategory().name())); } } @@ -262,7 +264,7 @@ public class RuleMapper { private static void setGapDescription(Rules.Rule.Builder ruleResponse, RuleDto ruleDto, Set fieldsToReturn) { String gapDescription = ruleDto.getGapDescription(); if (shouldReturnField(fieldsToReturn, FIELD_GAP_DESCRIPTION) - && gapDescription != null) { + && gapDescription != null) { ruleResponse.setGapDescription(gapDescription); } } @@ -389,6 +391,7 @@ public class RuleMapper { /** * This was done to preserve backward compatibility with SonarLint until they stop using htmlDesc field in api/rules/[show|search] endpoints, see SONAR-16635 + * * @deprecated the method should be removed once SonarLint supports rules.descriptionSections fields, I.E in 10.x and DB is cleaned up of non-necessary default sections. */ @Deprecated(since = "9.6", forRemoval = true) -- 2.39.5