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

Create rule CSS:S125: Sections of code should not be commented out #4338

Merged
merged 2 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion its/plugin/projects/css-issues-project/src/file2.less
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ a:unknown { /* S4659 | selecto
width: calc(100% 80px); /* S5362 | function-calc-no-invalid */
}

// color: pink; /* S4668 | no-invalid-double-slash-comments | Doesn't raise for LESS */
// color: pink; /* S4668 | no-invalid-double-slash-comments | Doesn't raise for LESS */ /* SS125 | no-commented-code */

.class1 {
width: 100px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ void issue_list() {
tuple("css:S5362", "css-issues-project:src/file1.css"),
tuple("css:S5362", "css-issues-project:src/file2.less"),
tuple("css:S5362", "css-issues-project:src/file3.scss"),
tuple("css:S1116", "css-issues-project:src/file5-1.html")
tuple("css:S1116", "css-issues-project:src/file5-1.html"),
tuple("css:S125", "css-issues-project:src/file2.less")
);
}
}
5 changes: 5 additions & 0 deletions its/ruling/src/test/expected/css/css-S125.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"project:custom/S125.css": [
3
]
}
4 changes: 4 additions & 0 deletions its/sources/css/custom/S125.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
p {
color: red;
/* font-size: large; */
}
25 changes: 8 additions & 17 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
"jsx-ast-utils": "3.3.5",
"lodash.clone": "4.5.0",
"module-alias": "2.2.3",
"postcss": "8.4.31",
"postcss-html": "0.36.0",
"postcss-less": "6.0.0",
"postcss-sass": "0.5.0",
Expand Down Expand Up @@ -129,6 +130,7 @@
"jsx-ast-utils",
"lodash.clone",
"module-alias",
"postcss",
"postcss-html",
"postcss-less",
"postcss-sass",
Expand Down
20 changes: 20 additions & 0 deletions packages/css/src/rules/S125/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
export { rule } from './rule';
72 changes: 72 additions & 0 deletions packages/css/src/rules/S125/rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
// https://sonarsource.github.io/rspec/#/rspec/S125/css

import * as stylelint from 'stylelint';
import { parse } from 'postcss';

const ruleName = 'no-commented-code';
const messages = { commentedCode: 'Remove this commented out code.' };

const ruleImpl: stylelint.RuleBase = () => {
return (root: any, result: any) => {
root.walkComments((comment: any) => {
const { text } = comment;
if (isLikelyCss(text)) {
try {
parse(text);
stylelint.utils.report({
ruleName,
result,
message: messages.commentedCode,
node: comment,
});
} catch {
/* syntax error */
}
}
});
};

function isLikelyCss(text: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to have an example of what each regex is trying to match, next to the description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

// Regular expression to match CSS selectors, properties, and values
// `<selector(s)> '{' <anything> '}'`
const ruleRegex = /([a-z0-9\s,.\-#:_]+)\{([^}]*)\}/i;

// Regular expression to match CSS declarations
// `<property> ':' <value> ';'`
const declRegex = /([a-z-]+)\s*:\s*([^;]+);/i;

// Regular expression to match CSS at-rules
// `'@' <at-rule> '(' <anything> ')'`
const atRuleRegex = /@([a-z-]*)\s*([^;{]*)(;|(\{([^}]*)\}))/i;

// Test the text against the regular expressions
return ruleRegex.test(text) || declRegex.test(text) || atRuleRegex.test(text);
}
};

export const rule = stylelint.createPlugin(
ruleName,
Object.assign(ruleImpl, {
messages,
ruleName,
}),
) as { ruleName: string; rule: stylelint.Rule };
103 changes: 103 additions & 0 deletions packages/css/src/rules/S125/unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import { StylelintRuleTester } from '../../../tests/tools/tester';
import { rule } from './rule';

const ruleTester = new StylelintRuleTester(rule);
ruleTester.run('no-commented-code', {
valid: [
{
description: 'no comment',
code: 'p {}',
},
{
description: 'no commented code',
code: '/* hello, world! */',
},
],
invalid: [
{
description: 'selector',
code: '/* p {} */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'multiple selectors',
code: '/* p, div {} */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'declaration',
code: '/* color: blue; */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'selector declaration',
code: '/* p { color: blue; } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'multiple declarations',
code: '/* div { font-size: 20px; color: red; } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'class selector',
code: '/* .class { background-color: red; } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'id selector',
code: '/* #id:hover { border: 1px solid black; } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'attribute selector',
code: '/* a[href] { color: purple; } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'media query',
code: '/* @media (max-width: 600px) { .class { font-size: 18px; } } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: '@keyframes',
code: '/* @keyframes mymove { 0% { top: 0px; } 100% { top: 200px; } } */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'import',
code: '/* @import url("styles.css"); */',
errors: [{ text: 'Remove this commented out code.', line: 1, column: 1 }],
},
{
description: 'multline',
code: `
/*
p {
color: blue;
}
*/
`,
errors: [{ text: 'Remove this commented out code.', line: 2, column: 1 }],
},
],
});
2 changes: 2 additions & 0 deletions packages/css/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import * as stylelint from 'stylelint';
import { rule as S125 } from './S125';
import { rule as S5362 } from './S5362';

/**
Expand All @@ -28,6 +29,7 @@ const rules: { [key: string]: stylelint.Rule } = {};
/**
* Maps Stylelint rule keys to rule implementations
*/
rules[S125.ruleName] = S125.rule; // no-commented-code
rules[S5362.ruleName] = S5362.rule; // function-calc-no-invalid

export { rules };
2 changes: 2 additions & 0 deletions sonar-plugin/css/src/main/java/org/sonar/css/CssRules.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.sonar.css.rules.FunctionLinearGradientNoNonstandardDirection;
import org.sonar.css.rules.KeyframeDeclarationNoImportant;
import org.sonar.css.rules.MediaFeatureNameNoUnknown;
import org.sonar.css.rules.NoCommentedCode;
import org.sonar.css.rules.NoDescendingSpecificity;
import org.sonar.css.rules.NoDuplicateAtImportRules;
import org.sonar.css.rules.NoDuplicateSelectors;
Expand Down Expand Up @@ -89,6 +90,7 @@ public static List<Class<?>> getRuleClasses() {
FunctionLinearGradientNoNonstandardDirection.class,
KeyframeDeclarationNoImportant.class,
MediaFeatureNameNoUnknown.class,
NoCommentedCode.class,
NoDescendingSpecificity.class,
NoDuplicateAtImportRules.class,
NoDuplicateSelectors.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.css.rules;

import org.sonar.check.Rule;

@Rule(key = "S125")
public class NoCommentedCode implements CssRule {

@Override
public String stylelintKey() {
return "no-commented-code";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<h2>Why is this an issue?</h2>
<p>Commented-out code distracts the focus from the actual executed code. It creates a noise that increases maintenance code. And because it is never
executed, it quickly becomes out of date and invalid.</p>
<p>Commented-out code should be deleted and can be retrieved from source control history if required.</p>

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Sections of code should not be commented out",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "CLEAR"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"unused"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-125",
"sqKey": "S125",
"scope": "All",
"quickfix": "unknown"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "Sonar way",
"ruleKeys": [
"S125",
"S1116",
"S1128",
"S4647",
Expand Down