Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule/erea001 supported version range #28

Merged
merged 3 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
<severity>MINOR</severity>
<name><![CDATA[Supported Version Range]]></name>
<internalKey><![CDATA[SupportedVersionRange]]></internalKey>
<description><![CDATA[<p>When looking at the minSdkVersion and targetSdkVersion attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.</p>
<description><![CDATA[<p>When looking at the minSdkVersion and targetSdkVersion (or minSdk, targetSdk) attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.</p>
<p>Example of violations:</p>
<pre><code>android {
compileSdk 32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,57 +18,75 @@
*/
package org.sonar.plugins.groovy;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.junit.Test;
import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition;
import org.sonar.api.server.profile.BuiltInQualityProfilesDefinition.BuiltInActiveRule;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.api.utils.ValidationMessages;
import org.sonar.plugins.groovy.codenarc.CodeNarcRulesDefinition;
import org.sonar.plugins.groovy.foundation.Groovy;
import org.w3c.dom.Document;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;

public class GroovySonarWayProfileTest {

@Test
public void shouldCreateSonarWayProfile() {
ValidationMessages messages = ValidationMessages.create();
@Test
public void shouldCreateSonarWayProfile() {
ValidationMessages messages = ValidationMessages.create();

GroovySonarWayProfile profileDef = new GroovySonarWayProfile();
BuiltInQualityProfilesDefinition.Context profileContext =
new BuiltInQualityProfilesDefinition.Context();
profileDef.define(profileContext);
BuiltInQualityProfilesDefinition.BuiltInQualityProfile profile =
profileContext.profile(Groovy.KEY, Groovy.PROFILE_NAME);
assertThat(profile.language()).isEqualTo(Groovy.KEY);
List<BuiltInActiveRule> activeRules = profile.rules();
// TODO The Number of Custom profile rules are set here,
// we need to change this to get the size dynamically
assertThat(activeRules).as("Expected number of rules in profile").hasSize(3);
assertThat(profile.name()).isEqualTo(Groovy.PROFILE_NAME);
GroovySonarWayProfile profileDef = new GroovySonarWayProfile();
BuiltInQualityProfilesDefinition.Context profileContext = new BuiltInQualityProfilesDefinition.Context();
profileDef.define(profileContext);
BuiltInQualityProfilesDefinition.BuiltInQualityProfile profile = profileContext.profile(Groovy.KEY, Groovy.PROFILE_NAME);
assertThat(profile.language()).isEqualTo(Groovy.KEY);
List<BuiltInActiveRule> activeRules = profile.rules();

// Check that we use severity from the read rule and not default one.
assertThat(activeRules.get(0).overriddenSeverity()).isNull();
assertThat(messages.hasErrors()).isFalse();
// Here we check the rules.xml file to get the number of rules in it
File profile_default = new File("src/main/resources/org/sonar/plugins/groovy/rules.xml");
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
DocumentBuilder db;
try {
db = dbf.newDocumentBuilder();
Document doc = db.parse(profile_default);
doc.getDocumentElement().normalize();
NodeList rule = doc.getElementsByTagName("rule");
assertThat(activeRules).as("Expected number of rules in profile").hasSize(rule.getLength());
} catch (ParserConfigurationException | SAXException | IOException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we have this this exception ? The assert l65 is skipped and we have no test failure I think. We must force a failure here.

}
assertThat(profile.name()).isEqualTo(Groovy.PROFILE_NAME);

// Check that all rules in "Sonar way" actually exist
CodeNarcRulesDefinition definition = new CodeNarcRulesDefinition();
RulesDefinition.Context rulesContext = new RulesDefinition.Context();
definition.define(rulesContext);
RulesDefinition.Repository repository =
rulesContext.repository(CodeNarcRulesDefinition.REPOSITORY_KEY);
// Check that we use severity from the read rule and not default one.
assertThat(activeRules.get(0).overriddenSeverity()).isNull();
assertThat(messages.hasErrors()).isFalse();

Set<String> rules = new HashSet<>();
for (RulesDefinition.Rule rule : repository.rules()) {
rules.add(rule.key());
}
for (BuiltInActiveRule activeRule : profile.rules()) {
assertThat(rules.contains(activeRule.ruleKey()))
.as("No such rule: " + activeRule.ruleKey())
.isTrue();
// Check that all rules in "Sonar way" actually exist
CodeNarcRulesDefinition definition = new CodeNarcRulesDefinition();
RulesDefinition.Context rulesContext = new RulesDefinition.Context();
definition.define(rulesContext);
RulesDefinition.Repository repository =
rulesContext.repository(CodeNarcRulesDefinition.REPOSITORY_KEY);

Set<String> rules = new HashSet<>();
for (RulesDefinition.Rule rule : repository.rules()) {
rules.add(rule.key());
}
for (BuiltInActiveRule activeRule : profile.rules()) {
assertThat(rules.contains(activeRule.ruleKey()))
.as("No such rule: " + activeRule.ruleKey())
.isTrue();
}
}
}
}
2 changes: 1 addition & 1 deletion codenarc-converter/CodeNarc/docs/codenarc-rules-ecocode.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Example of violations:

*Since CodeNarc 2.2.1*

When looking at the minSdkVersion and targetSdkVersion attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.
When looking at the minSdkVersion and targetSdkVersion (or minSdk, targetSdk) attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.

Example of violations:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.codenarc.rule.AbstractAstVisitorRule
import org.codenarc.util.AstUtil

/**
* When looking at the minSdkVersion and targetSdkVersion attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.
* When looking at the minSdkVersion and targetSdkVersion or minSdk and targetSdk attributes for the <uses-sdk> in the AndroidManifest.xml file, the amplitude of supported platform versions should not be too wide, at the risk of making the app too heavy to handle all cases.
*
* @author Leboulanger Mickael
*/
Expand All @@ -31,7 +31,9 @@ class SupportedVersionRangeRule extends AbstractAstVisitorRule {
String name = 'SupportedVersionRange'
int priority = 2
int minSdkVersion = 0
int minSdk = 0
int targetSdkVersion = 0
int targetSdk = 0
int threshold = 4 // Value used to compare minSdkVersion and targetSdkVersion
Class astVisitorClass = SupportedVersionRangeAstVisitor
}
Expand All @@ -41,14 +43,20 @@ class SupportedVersionRangeAstVisitor extends AbstractAstVisitor {
@Override
void visitMethodCallExpression(MethodCallExpression methodCallExpression) {
def methodName = ((ConstantExpression) methodCallExpression.getMethod()).getValue()
if (methodName == 'minSdkVersion' || methodName == 'targetSdkVersion') {
if (methodName == 'minSdkVersion' || methodName == 'targetSdkVersion' || methodName == 'minSdk' || methodName == 'targetSdk') {
def arguments = AstUtil.getArgumentsValue(methodCallExpression.getArguments())

if (arguments.size() == 1) {
rule[methodName] = arguments.get(0)
}

if ((rule.minSdkVersion && rule.targetSdkVersion) && rule.targetSdkVersion - rule.minSdkVersion > rule.threshold) {
addViolation(methodCallExpression, getViolationMessage())
}

if ((rule.minSdk && rule.targetSdk) && rule.targetSdk - rule.minSdk > rule.threshold) {
addViolation(methodCallExpression, getViolationMessage())
}
}
super.visitMethodCallExpression(methodCallExpression)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,41 @@ class SupportedVersionRangeRuleTest extends AbstractRuleTestCase<SupportedVersio
assertSingleViolation(SOURCE,8, 'targetSdkVersion')
}


@Test
void test_SdkRange_Short_SingleViolation() {
final SOURCE = '''
android {
compileSdk 32

defaultConfig {
applicationId "com.example.sampleForSonar"
minSdk 26
targetSdk 31
versionCode 1
versionName "1.0"

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
}

buildTypes {
release {
minifyEnabled false
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro\'
}
}
compileOptions {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
buildFeatures {
viewBinding true
}
}
'''
assertSingleViolation(SOURCE,8, 'targetSdk')
}

@Override
protected SupportedVersionRangeRule createRule() {
new SupportedVersionRangeRule()
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ services:
sonar:
build:
dockerfile: Dockerfile
context: .
container_name: sonar_ecocode-mobile
ports:
- "9000:9000"
Expand Down