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

Enable "error prone" library that does static code analysis during code compilation #421

Merged
merged 28 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
78b590e
[error-prone] add missing @Override annotations
asolntsev Oct 16, 2024
0988aaa
run all tests with IDEA, not Maven.
asolntsev Oct 16, 2024
def0ce5
[error-prone] fix all places where locale or charset was not specified
asolntsev Oct 16, 2024
90e93e9
[error-prone] remove unused `Node` parameter
asolntsev Oct 16, 2024
91294c7
[error-prone] remove unused classes
asolntsev Oct 16, 2024
6ca8743
[error-prone] simplify color calculation
asolntsev Oct 16, 2024
f2092bb
[error-prone] avoid static import when the imported name is too generic
asolntsev Oct 16, 2024
5f09736
[error-prone] cleanup class `Graphics2DRenderer`
asolntsev Oct 16, 2024
c3135cc
more nullabale / final fields
asolntsev Oct 16, 2024
24aaf37
[cleanup] remove unused methods
asolntsev Oct 16, 2024
7976c81
[error-prone] fix method `SwingReplacedElementFactory.remove(Element)`
asolntsev Oct 16, 2024
7785f59
include slf4j-api dependency in "*-examples" project
asolntsev Oct 16, 2024
6bb98c4
[cleanup] remove unused class NoReplacedElementFactory
asolntsev Oct 23, 2024
b34084f
[error-prone] add missing @Override annotations
asolntsev Oct 23, 2024
c7b606c
[error-prone] remove unused classes
asolntsev Oct 23, 2024
69c38b6
try to fix error-prone configuration
asolntsev Oct 23, 2024
4706487
[error-prone] more @Nullable fields
asolntsev Oct 23, 2024
181e348
[error-prone] fix error: "ParserTest.java:[75,34] [CheckReturnValue] …
asolntsev Oct 23, 2024
77027bd
[error-prone] close I/O resources properly
asolntsev Oct 23, 2024
fa617c1
[error-prone] improve javadoc
asolntsev Oct 23, 2024
0e43113
[error-prone] LinkedList -> ArrayList
asolntsev Oct 23, 2024
859dd76
[error-prone] avoid using default system locale
asolntsev Oct 23, 2024
ff092ae
[error-prone] avoid using default system charset
asolntsev Oct 23, 2024
543e93b
[error-prone] remove unused parameters
asolntsev Oct 24, 2024
0cf667d
[error-prone] fix object comparision by reference
asolntsev Oct 24, 2024
b4c7b4a
[error-prone] use `RenderingHints.Key` instead of just `Key` for bett…
asolntsev Oct 24, 2024
2ea5497
[error-prone] return more generic `Set` instead of more specific `Tre…
asolntsev Oct 24, 2024
f490eb3
[error-prone] mark as OK few switches that do not cover all branches
asolntsev Oct 24, 2024
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
4 changes: 4 additions & 0 deletions .idea/inspectionProfiles/Project_Default.xml

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

10 changes: 10 additions & 0 deletions .mvn/jvm.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
--add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
--add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED
42 changes: 13 additions & 29 deletions .run/test.run.xml
Original file line number Diff line number Diff line change
@@ -1,32 +1,16 @@
<component name="ProjectRunConfigurationManager">
<configuration default="false" name="test" type="MavenRunConfiguration" factoryName="Maven">
<MavenSettings>
<option name="myGeneralSettings" />
<option name="myRunnerSettings" />
<option name="myRunnerParameters">
<MavenRunnerParameters>
<option name="cmdOptions" />
<option name="profiles">
<set />
</option>
<option name="goals">
<list>
<option value="test" />
</list>
</option>
<option name="multimoduleDir" />
<option name="pomFileName" />
<option name="profilesMap">
<map />
</option>
<option name="projectsCmdOptionValues">
<list />
</option>
<option name="resolveToWorkspace" value="false" />
<option name="workingDirPath" value="$PROJECT_DIR$" />
</MavenRunnerParameters>
</option>
</MavenSettings>
<method v="2" />
<configuration default="false" name="test" type="JUnit" factoryName="JUnit">
<option name="ALTERNATIVE_JRE_PATH_ENABLED" value="true" />
<option name="ALTERNATIVE_JRE_PATH" value="17" />
<option name="PACKAGE_NAME" value="org.xhtmlrenderer" />
<option name="MAIN_CLASS_NAME" value="" />
<option name="METHOD_NAME" value="" />
<option name="TEST_OBJECT" value="package" />
<option name="TEST_SEARCH_SCOPE">
<value defaultName="wholeProject" />
</option>
<method v="2">
<option name="Make" enabled="true" />
</method>
</configuration>
</component>
8 changes: 0 additions & 8 deletions flying-saucer-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@
</instructions>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>${java.version}</source>
<target>${java.version}</target>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
*/
package org.xhtmlrenderer.context;

import org.w3c.dom.css.CSSPrimitiveValue;
import com.google.errorprone.annotations.CheckReturnValue;
import org.jspecify.annotations.Nullable;
import org.xhtmlrenderer.css.constants.IdentValue;
import org.xhtmlrenderer.css.extend.ContentFunction;
import org.xhtmlrenderer.css.parser.FSFunction;
Expand All @@ -35,6 +36,10 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import static org.w3c.dom.css.CSSPrimitiveValue.CSS_IDENT;
import static org.w3c.dom.css.CSSPrimitiveValue.CSS_STRING;

public class ContentFunctionFactory {
private final List<ContentFunction> _functions = new ArrayList<>();
Expand All @@ -46,6 +51,7 @@ public ContentFunctionFactory() {
_functions.add(new LeaderFunction());
}

@Nullable
public ContentFunction lookupFunction(LayoutContext c, FSFunction function) {
for (ContentFunction f : _functions) {
if (f.canHandle(c, function)) {
Expand All @@ -65,6 +71,7 @@ public boolean isStatic() {
return false;
}

@Nullable
@Override
public String calculate(LayoutContext c, FSFunction function) {
return null;
Expand Down Expand Up @@ -95,14 +102,14 @@ protected boolean isCounter(FSFunction function, String counterName) {
List<PropertyValue> parameters = function.getParameters();
if (parameters.size() == 1 || parameters.size() == 2) {
PropertyValue param = parameters.get(0);
if (param.getPrimitiveType() != CSSPrimitiveValue.CSS_IDENT ||
! param.getStringValue().equals(counterName)) {
if (param.getPrimitiveType() != CSS_IDENT ||
!Objects.equals(param.getStringValue(), counterName)) {
return false;
}

if (parameters.size() == 2) {
param = parameters.get(1);
return param.getPrimitiveType() == CSSPrimitiveValue.CSS_IDENT;
return param.getPrimitiveType() == CSS_IDENT;
}

return true;
Expand Down Expand Up @@ -163,6 +170,7 @@ public String calculate(RenderingContext c, FSFunction function, InlineText text
return "";
}

@Nullable
@Override
public String calculate(LayoutContext c, FSFunction function) {
return null;
Expand All @@ -181,14 +189,14 @@ public boolean canHandle(LayoutContext c, FSFunction function) {
FSFunction f = parameters.get(0).getFunction();
if (f == null ||
f.getParameters().size() != 1 ||
f.getParameters().get(0).getPrimitiveType() != CSSPrimitiveValue.CSS_IDENT ||
! f.getParameters().get(0).getStringValue().equals("href")) {
f.getParameters().get(0).getPrimitiveType() != CSS_IDENT ||
! "href".equals(f.getParameters().get(0).getStringValue())) {
return false;
}

PropertyValue param = parameters.get(1);
return param.getPrimitiveType() == CSSPrimitiveValue.CSS_IDENT &&
param.getStringValue().equals("page");
return param.getPrimitiveType() == CSS_IDENT &&
Objects.equals(param.getStringValue(), "page");
}
}

Expand Down Expand Up @@ -253,10 +261,12 @@ public String calculate(RenderingContext c, FSFunction function, InlineText text
return leaderString;
}

@Nullable
@CheckReturnValue
private String getLeaderValue(FSFunction function) {
final PropertyValue param = function.getParameters().get(0);
final String value = param.getStringValue();
if (param.getPrimitiveType() == CSSPrimitiveValue.CSS_IDENT) {
if (param.getPrimitiveType() == CSS_IDENT) {
return switch (value) {
case "dotted" -> ". ";
case "solid" -> "_";
Expand All @@ -267,27 +277,30 @@ private String getLeaderValue(FSFunction function) {
return value;
}

@Nullable
@Override
public String calculate(LayoutContext c, FSFunction function) {
return null;
}

@Override
@CheckReturnValue
public String getLayoutReplacementText() {
return " . ";
}

@Override
@CheckReturnValue
public boolean canHandle(LayoutContext c, FSFunction function) {
if (c.isPrint() && function.is("leader")) {
List<PropertyValue> parameters = function.getParameters();
if (parameters.size() == 1) {
PropertyValue param = parameters.get(0);
return param.getPrimitiveType() == CSSPrimitiveValue.CSS_STRING ||
(param.getPrimitiveType() == CSSPrimitiveValue.CSS_IDENT &&
(param.getStringValue().equals("dotted") ||
param.getStringValue().equals("solid") ||
param.getStringValue().equals("space")));
return param.getPrimitiveType() == CSS_STRING ||
(param.getPrimitiveType() == CSS_IDENT &&
("dotted".equals(param.getStringValue()) ||
"solid".equals(param.getStringValue()) ||
"space".equals(param.getStringValue())));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,17 @@
* @author Torbjoern Gannholm
*/
public class StyleReference {
@Nullable
private NamespaceHandler _nsh;
@Nullable
private Document _doc;
private final StylesheetFactoryImpl _stylesheetFactory;

/**
* Instance of our element-styles matching class. Will be null if new rules
* have been added since last match.
*/
@Nullable
private Matcher _matcher;

private UserAgentCallback _uac;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.xhtmlrenderer.css.sheet.Stylesheet;

import java.util.LinkedHashMap;
import java.util.Map;

class StylesheetCache extends LinkedHashMap<String, Stylesheet> {
private static final int cacheCapacity = 16;
Expand All @@ -11,7 +12,8 @@ class StylesheetCache extends LinkedHashMap<String, Stylesheet> {
super(cacheCapacity, 0.75f, true);
}

protected boolean removeEldestEntry(java.util.Map.Entry<String, Stylesheet> eldest) {
@Override
protected boolean removeEldestEntry(Map.Entry<String, Stylesheet> eldest) {
return size() > cacheCapacity;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.xhtmlrenderer.extend.UserAgentCallback;
import org.xhtmlrenderer.resource.CSSResource;
import org.xhtmlrenderer.util.Configuration;
import org.xhtmlrenderer.util.IOUtil;
import org.xhtmlrenderer.util.XRLog;
import org.xml.sax.InputSource;

Expand All @@ -39,7 +38,6 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.UnsupportedEncodingException;
import java.util.Map;
import java.util.logging.Level;

Expand Down Expand Up @@ -70,10 +68,12 @@ public StylesheetFactoryImpl(UserAgentCallback userAgentCallback) {
_cssParser = new CSSParser((uri, message) -> XRLog.cssParse(Level.WARNING, "(" + uri + ") " + message));
}

@Override
public Stylesheet parse(Reader reader, StylesheetInfo info) {
return parse(reader, info.getUri(), info.getOrigin());
}

@Override
public Stylesheet parse(Reader reader, String uri, Origin origin) {
try {
return _cssParser.parseStylesheet(uri, origin, reader);
Expand All @@ -96,19 +96,16 @@ private Stylesheet parse(StylesheetInfo info) {
// since the null resource stream is wrapped in a BufferedInputStream
InputSource inputSource=cr.getResourceInputSource();
if (inputSource==null) return null;
InputStream is = inputSource.getByteStream();
if (is==null) return null;
try {
try (InputStream is = inputSource.getByteStream()) {
if (is == null) return null;
String charset = Configuration.valueFor("xr.stylesheets.charset-name", "UTF-8");
return parse(new InputStreamReader(is, charset), info);
} catch (UnsupportedEncodingException e) {
// Shouldn't happen
} catch (IOException e) {
throw new RuntimeException(e.getMessage(), e);
} finally {
IOUtil.close(is);
}
}

@Override
public Ruleset parseStyleDeclaration(Origin origin, String styleDeclaration) {
return _cssParser.parseDeclaration(origin, styleDeclaration);
}
Expand Down Expand Up @@ -158,6 +155,7 @@ void flushCachedStylesheets() {
//TODO: this looks a bit odd
@Nullable
@CheckReturnValue
@Override
public Stylesheet getStylesheet(StylesheetInfo info) {
XRLog.load("Requesting stylesheet: " + info.getUri());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,7 @@ private CSSName(
*
* @return a string representation of the object.
*/
@Override
public String toString() {
return this.propName;
}
Expand Down Expand Up @@ -1827,14 +1828,15 @@ public int compareTo(CSSName object) {
}

// FIXME equals, hashcode

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof CSSName cssName)) return false;

return FS_ID == cssName.FS_ID;
}

@Override
public int hashCode() {
return FS_ID;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ private IdentValue(String ident) {
*
* @return a string representation of the object.
*/
@Override
public String toString() {
return ident;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ private static MarginBoxName addValue(String ident, IdentValue textAlign, IdentV
return val;
}

@Override
public String toString() {
return _ident;
}
Expand All @@ -76,10 +77,12 @@ public static MarginBoxName valueOf(String ident) {
return ALL.get(ident);
}

@Override
public int hashCode() {
return FS_ID;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof MarginBoxName)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public static PageElementPosition byIdent(String ident) {
this._ident = ident;
}

@Override
public String toString() {
return _ident;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import org.w3c.dom.Node;

/**
* Gives the css matcher access to the information it needs about the tree structure.
* <p>
* Elements are the "things" in the tree structure that can be matched by the matcher.
*
* @author scott
* <p>
* Gives the css matcher access to the information it needs about the tree structure.
* <p>
* Elements are the "things" in the tree structure that can be matched by the matcher.
*/
public interface TreeResolver {
// XXX Where should this go (used by parser, TreeResolver, and AttributeResolver
Expand Down
Loading