-
Notifications
You must be signed in to change notification settings - Fork 79
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
[JAVA] [EC10] Image vectors #154
base: main
Are you sure you want to change the base?
Changes from all commits
d7d50a6
fed9970
5e56b8a
5467a3a
8967824
eb3d73d
96800a7
e5af3ad
e952e8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
package fr.greencodeinitiative.java.checks; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indeed, this new rule implementation isn't included in ecoCode plugin because there is no declaration in |
||
|
||
import org.sonar.check.Priority; | ||
import org.sonar.check.Rule; | ||
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||
import org.sonar.plugins.java.api.tree.*; | ||
import org.w3c.dom.*; | ||
import org.xml.sax.InputSource; | ||
import org.xml.sax.SAXException; | ||
|
||
import javax.xml.parsers.DocumentBuilder; | ||
import javax.xml.parsers.DocumentBuilderFactory; | ||
import javax.xml.parsers.ParserConfigurationException; | ||
import javax.xml.transform.Transformer; | ||
import javax.xml.transform.TransformerConfigurationException; | ||
import javax.xml.transform.TransformerException; | ||
import javax.xml.transform.TransformerFactory; | ||
import javax.xml.transform.dom.DOMSource; | ||
import javax.xml.transform.stream.StreamResult; | ||
import javax.xml.xpath.XPath; | ||
import javax.xml.xpath.XPathConstants; | ||
import javax.xml.xpath.XPathExpressionException; | ||
import javax.xml.xpath.XPathFactory; | ||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.StringReader; | ||
import java.lang.reflect.Field; | ||
import java.rmi.UnmarshalException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.stream.Stream; | ||
|
||
@Rule( | ||
key = "EC53", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please, check other rule implementation to make the same :
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong rule id ==> |
||
name = "Developpement", | ||
description = OptimizeImageVectorsSize.MESSAGERULE, | ||
priority = Priority.MINOR, | ||
tags = {"bug"}) | ||
public class OptimizeImageVectorsSize extends IssuableSubscriptionVisitor { | ||
protected final String SVG_BEGIN = "<svg"; | ||
protected final String[] SVG_END = new String[]{"</svg", "</ svg>"}; | ||
protected final List<String> SUPERFLUOUS_ATTRIBUTES = Arrays.asList("xmlns:dc", "xmlns:cc", "xmlns:rdv", "xmlns:svg", "xmlns", "xmlns:sodipodi", "xmlns:inkscape", "inkscape:version", "inkscape:label", "inkscape:groupmode", "sodipodi:docname"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please could you tell us where do you find this list of superfluous attributes ? I'm curious :p There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello @dedece35 You can also define custom attributes in svg, e.g: version='1.2' |
||
protected static final String MESSAGERULE = "Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner"; | ||
// COMPLETE MESSAGE: | ||
// "Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner. The following should be avoided: The attribute value length can be reduced, e.g: id=\"filter4210\" to id=\"a\" (the same goes for xlink:href attribute). The attributes' value should be approximated: stdDeviation=\"0.57264819\" to stdDeviation=\"0.573\". Duplicated tags. Superfluous tags, e.g: <sodipodi:namedview>, <metadata>. Redundant tags. Superfluous attributes (xmlns:dc, xmlns:cc, xmlns:rdv, xmlns: svg, xmlns, xmlns:sodipodi, xmlns:inkscape, most of id attributes, version, inkscape:version, inkscape:label, inkscape:groupmode, sodipodi:docname)"; | ||
|
||
@Override | ||
public List<Tree.Kind> nodesToVisit() { | ||
return List.of(Tree.Kind.STRING_LITERAL, Tree.Kind.TEXT_BLOCK); | ||
} | ||
|
||
@Override | ||
public void visitNode(Tree tree) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please, explode this method in several smaller private functional methods. this is for readability and ease to maintain it. |
||
String value = ((LiteralTree) tree).value(); | ||
|
||
// convert to lower case (not case sensitve anymore) | ||
value = value.toLowerCase(); | ||
|
||
// trim a beginning and ending double quotes (") and single quotes (') | ||
if (tree.is(Tree.Kind.STRING_LITERAL)) { | ||
value = value.substring(1, value.length() - 1); | ||
} else { | ||
// a text block begins and ends with 3 double quotes | ||
value = value.substring(3, value.length() - 3); | ||
} | ||
|
||
// stop escaping double quotes (") and single quotes (') | ||
value = value.replaceAll("\\\\\"", "\""); | ||
value = value.replaceAll("\\\\\'", "\'"); | ||
|
||
// search for svg beginning tag | ||
int beginIndex = value.indexOf(SVG_BEGIN); | ||
if (beginIndex < 0) { | ||
// the string doesn't contain any svg | ||
return; | ||
} | ||
|
||
// search for svg ending tag | ||
int endIndex = -1; | ||
int j = 0; | ||
while (j < SVG_END.length && endIndex == -1) { | ||
endIndex = value.indexOf(SVG_END[j]); | ||
++j; | ||
} | ||
if (endIndex == -1) { | ||
return; // svg is invalid or is broken into multiple strings | ||
} | ||
|
||
// Parse svg as xml and explore its tree | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you use XML parsing and not simplier way, for example string pattern search on all java code ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into regex matching at first, but I realized it may not suit some rules I defined. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should consider using XPath? SVGs files being XML, this would allow us to perform the analysis independently of what language/framework is used. This is mentionned in the SonarQube documentation |
||
try { | ||
// Build the doc from the XML file | ||
InputSource source = new InputSource(new StringReader(value)); | ||
|
||
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); | ||
DocumentBuilder db = dbf.newDocumentBuilder(); | ||
Document doc = db.parse(source); | ||
|
||
// the string is a valid xml file | ||
|
||
XPath xpath = XPathFactory.newInstance().newXPath(); | ||
|
||
// Note to developers: check xpath expressions by using xpather: http://xpather.com/ | ||
|
||
// Superfluous tags, e.g: <sodipodi:namedview>, <metadata> | ||
if (((Double) xpath.evaluate("count(*//sodipodi:namedview)", | ||
doc, XPathConstants.NUMBER)).intValue() > 0 || ((Double) xpath.evaluate("count(*//metadata)", | ||
doc, XPathConstants.NUMBER)).intValue() > 0) { | ||
reportIssue(tree, MESSAGERULE); | ||
return; | ||
} | ||
|
||
int nbId = ((Double) xpath.evaluate("count(//@id)", doc, XPathConstants.NUMBER)).intValue(); | ||
int nbDistinctId = ((Double) xpath.evaluate("count(//@id[not(. = preceding::*/@id)])", doc, XPathConstants.NUMBER)).intValue(); | ||
int nbHref = ((Double) xpath.evaluate("count(//@xlink:href)", doc, XPathConstants.NUMBER)).intValue(); | ||
int nbDistinctHref = ((Double) xpath.evaluate("count(//@xlink:href[not(. = preceding::*/@xlink:href)])", doc, XPathConstants.NUMBER)).intValue(); | ||
|
||
// Duplicated tags (tags with the same xlink:href or same id attributes) | ||
if (nbId != nbDistinctId || nbHref != nbDistinctHref) { | ||
// count(//@id) returns the number of elements having an attribute "id" | ||
// count(//@id[not(. = preceding::*/@id)]) returns the number of unique "id" attribute values | ||
// if the number of "id" attributes and the number of unique "id" attributes values | ||
// aren't equal, that means at least two elements have an "id" attribute. | ||
reportIssue(tree, MESSAGERULE); | ||
return; | ||
} | ||
|
||
// The attribute value length can be reduced | ||
if ((boolean) xpath.evaluate("boolean(//@id[string-length() > 3])", | ||
doc, XPathConstants.BOOLEAN)) { | ||
reportIssue(tree, MESSAGERULE); | ||
return; | ||
} | ||
|
||
// The attributes' value should be approximated (there are attributes with a numeric value that have more than 3 decimals) | ||
// @* selects all the attributes of the document | ||
// [number(.) = .] is a predicate that filters all attributes whose value can be converted to a numeric one. | ||
if ((boolean) xpath.evaluate("boolean(//@*[number(.) = . and contains(., '.') and string-length(substring-after(., '.')) > 3])", doc, XPathConstants.BOOLEAN)) { | ||
reportIssue(tree, MESSAGERULE); | ||
return; | ||
} | ||
|
||
// Avoid superfluous attributes | ||
// We do not search for the superfluous attributes directly in the xml as a string as the attributes could be the text inside of xml tags. | ||
NodeList list = (NodeList) xpath.evaluate("//*", doc, XPathConstants.NODESET); | ||
int nbNodes = list.getLength(); | ||
for (int i = 0; i < nbNodes; ++i) { | ||
NamedNodeMap attributes = list.item(i).getAttributes(); | ||
int nbAttr = attributes.getLength(); | ||
for (j = 0; j < nbAttr; ++j) { | ||
if (SUPERFLUOUS_ATTRIBUTES.contains(((Attr) attributes.item(j)).getName())) { | ||
reportIssue(tree, MESSAGERULE); | ||
return; | ||
} | ||
} | ||
} | ||
|
||
} catch (Throwable ignored) { | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import java.util.Arrays; | ||
import java.util.Collection; | ||
import java.util.Collections; | ||
|
||
class TestClass { | ||
|
||
TestClass(TestClass tc) { | ||
} | ||
|
||
String compliantCase() { | ||
return "<svg id=\"a\"></svg>"; | ||
} | ||
|
||
String idCanBeReduced() { | ||
return "<svg id=\"main-wrapper\"></svg>"; // Noncompliant {{Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner}} | ||
} | ||
|
||
String superfluousTag() { | ||
return "<svg><metadata></metadata></svg>"; // Noncompliant {{Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner}} | ||
} | ||
|
||
String duplicatedTags() { | ||
return "<svg><linearGradient inkscape:collect=\"always\" xlink:href=\"#linearGradient4405\" id=\"linearGradient2917\" gradientUnits=\"userSpaceOnUse\" gradientTransform=\"matrix(19.565,0,0,19.565,-95.619,79.961)\" x1=\"11.685\" y1=\"33.646\" x2=\"10.783\" y2=\"35.870\" /><linearGradient inkscape:collect=\"always\" xlink:href=\"#linearGradient4405\" id=\"linearGradient2917\" gradientUnits=\"userSpaceOnUse\" gradientTransform=\"matrix(19.565,0,0,19.565,-95.619,79.961)\" x1=\"11.685\" y1=\"33.646\" x2=\"10.783\" y2=\"35.870\" /></svg>"; // Noncompliant {{Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner}} | ||
} | ||
|
||
String unapproximatedNumericValues() { | ||
return "<svg><linearGradient inkscape:collect=\"always\" xlink:href=\"#linearGradient4405\" id=\"linearGradient2917\" gradientUnits=\"userSpaceOnUse\" gradientTransform=\"matrix(19.565616,0,0,19.565616,-95.619433,79.96141)\" x1=\"11.685\" y1=\"33.646999\" x2=\"10.783\" y2=\"35.870998\" /></svg>"; // Noncompliant {{Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner}} | ||
} | ||
|
||
String validAttribute() { | ||
return "<svg><div trucmuche=\"test\"></div></svg>"; | ||
} | ||
|
||
String superfluousAttribute() { | ||
return "<svg><div xmlns:dc=\"test\"></div></svg>"; // Noncompliant {{Optimize your svg by using open source tools such as SVGGO, Compressor.io or SVG Cleaner}} | ||
} | ||
|
||
String blockCompliantCase() { | ||
return """ | ||
<svg id="a"></svg> | ||
"""; | ||
} | ||
|
||
/* can't write test as all comments are included in the text block and blocks the xml analysis | ||
String blockIdCanBeReduced() { | ||
return """ | ||
<svg id=\"main-wrapper\"></svg> | ||
"""; | ||
} | ||
*/ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package fr.greencodeinitiative.java.checks; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.sonar.java.checks.verifier.CheckVerifier; | ||
|
||
class OptimizeImageVectorsSizeTest { | ||
|
||
@Test | ||
void test() { | ||
CheckVerifier.newVerifier() | ||
.onFile("src/test/files/OptimizeImageVectorsSize.java") | ||
.withCheck(new OptimizeImageVectorsSize()) | ||
.verifyIssues(); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the use case for this need ?
for me, if container crashes, the developer must check why on logs et make a correction or an issue for correction.
if we add this option, we could miss the real pb