Skip to content

Commit

Permalink
See #23218: Use newer error_prone versions when compiling on Java 11+
Browse files Browse the repository at this point in the history
error_prone 2.11 dropped support for compiling with Java 8, although it still
supports compiling for Java 8. The "major" new check for us is `NotJavadoc` since
we used `/**` in quite a few places which were not javadoc.

Other "new" checks that are of interest:
* `AlreadyChecked`: `if (foo) { doFoo(); } else if (!foo) { doBar(); }`
* `UnnecessaryStringBuilder`: Avoid `StringBuilder` (Java converts `+` to
  `StringBuilder` behind-the-scenes, but may also do something else if it
  performs better)
* `NonApiType`: Avoid specific interface types in function definitions
* `NamedLikeContextualKeyword`: Avoid using restricted names for classes and
  methods
* `UnusedMethod`: Unused private methods should be removed

This fixes most of the new error_prone issues and some SonarLint issues.

git-svn-id: https://josm.openstreetmap.de/svn/trunk@18871 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
taylor.smock committed Oct 16, 2023
1 parent a9f8d35 commit 48f9f83
Show file tree
Hide file tree
Showing 62 changed files with 555 additions and 535 deletions.
8 changes: 7 additions & 1 deletion build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ Build-Date: ${build.tstamp}
<attribute name="release" default="${java.lang.version}"/>
<element name="cp-elements" optional="true"/>
<sequential>
<!-- RestrictedApiChecker was removed in error-prone 2.19 -->
<local name="errorProne2.10"/>
<property name="errorProne2.10" value="-Xep:RestrictedApiChecker:OFF" unless:set="isJava11"/>
<local name="errorProne2.22+"/>
<!-- LongDoubleConversion is disabled since SonarLint java:S1905 conflicts -->
<property name="errorProne2.22+" value="-Xep:LongDoubleConversion:OFF" if:set="isJava11"/>
<javac sourcepath="@{sourcepath}" srcdir="@{srcdir}" fork="@{fork}"
includes="@{includes}" excludes="@{excludes}" destdir="@{destdir}" release="@{release}"
debug="@{debug}" includeantruntime="@{includeantruntime}" encoding="@{encoding}">
Expand Down Expand Up @@ -249,7 +255,7 @@ Build-Date: ${build.tstamp}
<compilerarg value="-Xlint:unchecked"/>
<!-- Undocumented argument to ignore "Sun internal proprietary API" warning, see http://stackoverflow.com/a/13862308/2257172 -->
<compilerarg value="-XDignore.symbol.file"/>
<compilerarg value="-Xplugin:ErrorProne -XepExcludedPaths:.*/parsergen/.* -Xep:ReferenceEquality:OFF -Xep:FutureReturnValueIgnored:OFF -Xep:JdkObsolete:OFF -Xep:EqualsGetClass:OFF -Xep:UndefinedEquals:OFF -Xep:BadImport:OFF -Xep:AnnotateFormatMethod:OFF -Xep:JavaUtilDate:OFF -Xep:DoNotCallSuggester:OFF -Xep:BanSerializableRead:OFF -Xep:RestrictedApiChecker:OFF -Xep:InlineMeSuggester:OFF" unless:set="noErrorProne"/>
<compilerarg value="-Xplugin:ErrorProne -XepExcludedPaths:.*/parsergen/.* -Xep:ReferenceEquality:OFF -Xep:FutureReturnValueIgnored:OFF -Xep:JdkObsolete:OFF -Xep:EqualsGetClass:OFF -Xep:UndefinedEquals:OFF -Xep:BadImport:OFF -Xep:AnnotateFormatMethod:OFF -Xep:JavaUtilDate:OFF -Xep:DoNotCallSuggester:OFF -Xep:BanSerializableRead:OFF ${errorProne2.10} -Xep:InlineMeSuggester:OFF ${errorProne2.22+}" unless:set="noErrorProne"/>
<compilerarg line="-Xmaxwarns 1000"/>
<compilerarg value="-Xplugin:semanticdb -sourceroot:@{srcdir} -targetroot:${build.dir}/semanticdb" if:set="lsif" />
<classpath>
Expand Down
3 changes: 3 additions & 0 deletions ivysettings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@
<resolvers>
<ibiblio name="josm-nexus" m2compatible="true" root="https://josm.openstreetmap.de/nexus/content/repositories/public/" />
</resolvers>
<!-- Remove error_prone 2.10.0 specific statements in build.xml when we drop Java 8 as a build platform -->
<property name="versions.error_prone" value="2.10.0" unlessset="isJava11"/>
<property name="versions.error_prone" value="2.22.0" ifset="isJava11"/>
</ivysettings>
9 changes: 5 additions & 4 deletions src/org/openstreetmap/josm/actions/CloseChangesetAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

/**
* User action to close open changesets.
*
* <p>
* The list of open changesets will be downloaded from the server and presented
* to the user.
*/
Expand Down Expand Up @@ -109,11 +109,12 @@ protected void cancel() {
@Override
protected void finish() {
SwingUtilities.invokeLater(() -> {
if (lastException != null) {
ExceptionDialogUtil.explainException(lastException);
Exception exception = getLastException();
if (exception != null) {
ExceptionDialogUtil.explainException(exception);
}
ChangesetCache.getInstance().update(changesets);
if (!canceled && lastException == null) {
if (!isCanceled() && exception == null) {
onPostDownloadOpenChangesets();
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/org/openstreetmap/josm/actions/JoinAreasAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public AssembledMultipolygon(AssembledPolygon way) {
}

/**
* This hepler class implements algorithm traversing trough connected ways.
* This helper class implements algorithm traversing through connected ways.
* Assumes you are going in clockwise orientation.
* @author viesturs
*/
Expand Down
4 changes: 2 additions & 2 deletions src/org/openstreetmap/josm/actions/OpenFileAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ protected void alertFilesWithUnknownImporter(Collection<File> files) {
protected void realRun() throws SAXException, IOException, OsmTransferException {
if (Utils.isEmpty(files)) return;

/**
/*
* Find the importer with the chosen file filter
*/
FileImporter chosenImporter = null;
Expand All @@ -285,7 +285,7 @@ protected void realRun() throws SAXException, IOException, OsmTransferException
}
}
}
/**
/*
* If the filter hasn't been changed in the dialog, chosenImporter is null now.
* When the filter has been set explicitly to AllFormatsImporter, treat this the same.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private static List<String> paramCleanup(Collection<String> params) {
* @return map that maps shortened name to full directory path
*/
static Map<String, String> getAnonimicDirectorySymbolMap() {
/** maps the anonymized name to the actual used path */
/* maps the anonymized name to the actual used path */
Map<String, String> map = new LinkedHashMap<>();
map.put(PlatformManager.isPlatformWindows() ? "%JAVA_HOME%" : "${JAVA_HOME}", getSystemEnv("JAVA_HOME"));
map.put("<java.home>", getSystemProperty("java.home"));
Expand Down
10 changes: 5 additions & 5 deletions src/org/openstreetmap/josm/actions/UploadAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,27 @@ public class UploadAction extends AbstractUploadAction {
private static final String IS_ASYNC_UPLOAD_ENABLED = "asynchronous.upload";

static {
/**
/*
* Calls validator before upload.
*/
UPLOAD_HOOKS.add(new ValidateUploadHook());

/**
/*
* Fixes database errors
*/
UPLOAD_HOOKS.add(new FixDataHook());

/**
/*
* Checks server capabilities before upload.
*/
UPLOAD_HOOKS.add(new ApiPreconditionCheckerHook());

/**
/*
* Adjusts the upload order of new relations
*/
UPLOAD_HOOKS.add(new RelationUploadOrderHook());

/**
/*
* Removes discardable tags like created_by on modified objects
*/
LATE_UPLOAD_HOOKS.add(new DiscardTagsHook());
Expand Down
53 changes: 24 additions & 29 deletions src/org/openstreetmap/josm/actions/mapmode/ParallelWayAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,35 +55,30 @@

/**
* MapMode for making parallel ways.
*
* <p>
* All calculations are done in projected coordinates.
*
* <p>
* TODO:
* <p>
* == Functionality ==
*
* 1. Use selected nodes as split points for the selected ways.
*
* <ol>
* <li>Use selected nodes as split points for the selected ways.
* <p>
* The ways containing the selected nodes will be split and only the "inner"
* parts will be copied
*
* 2. Enter exact offset
*
* 3. Improve snapping
*
* 4. Visual cues could be better
*
* 5. (long term) Parallelize and adjust offsets of existing ways
*
* parts will be copied</li>
* <li>Enter exact offset</li>
* <li>Improve snapping</li>
* <li>Visual cues could be better</li>
* <li>(long term) Parallelize and adjust offsets of existing ways</li>
* </ol>
* == Code quality ==
*
* a) The mode, flags, and modifiers might be updated more than necessary.
*
* Not a performance problem, but better if they where more centralized
*
* b) Extract generic MapMode services into a super class and/or utility class
*
* c) Maybe better to simply draw our own source way highlighting?
*
* <ol type="a">
* <li>The mode, flags, and modifiers might be updated more than necessary.
* <p>
* Not a performance problem, but better if they where more centralized</li>
* <li>Extract generic MapMode services into a super class and/or utility class</li>
* <li>Maybe better to simply draw our own source way highlighting?</li>
* </ol>
* Current code doesn't not take into account that ways might been highlighted
* by other than us. Don't think that situation should ever happen though.
*
Expand Down Expand Up @@ -351,7 +346,7 @@ private static void removeWayHighlighting(Collection<Way> ways) {

@Override
public void mouseDragged(MouseEvent e) {
// WTF.. the event passed here doesn't have button info?
// WTF... the event passed here doesn't have button info?
// Since we get this event from other buttons too, we must check that
// _BUTTON1_ is down.
if (!mouseIsDown)
Expand Down Expand Up @@ -455,7 +450,7 @@ private boolean matchesCurrentModifiers(Map<Modifier, Boolean> spec) {
if (shift) {
modifiers.add(Modifier.SHIFT);
}
return spec.entrySet().stream().allMatch(entry -> modifiers.contains(entry.getKey()) == entry.getValue().booleanValue());
return spec.entrySet().stream().allMatch(entry -> modifiers.contains(entry.getKey()) == entry.getValue());
}

private boolean isModifiersValidForDragMode() {
Expand All @@ -464,11 +459,11 @@ private boolean isModifiersValidForDragMode() {
}

private void updateFlagsOnlyChangeableOnPress() {
copyTags = COPY_TAGS_DEFAULT.get().booleanValue() != matchesCurrentModifiers(COPY_TAGS_MODIFIER_COMBO);
copyTags = COPY_TAGS_DEFAULT.get() != matchesCurrentModifiers(COPY_TAGS_MODIFIER_COMBO);
}

private void updateFlagsChangeableAlways() {
snap = SNAP_DEFAULT.get().booleanValue() != matchesCurrentModifiers(SNAP_MODIFIER_COMBO);
snap = SNAP_DEFAULT.get() != matchesCurrentModifiers(SNAP_MODIFIER_COMBO);
}

// We keep the source ways and the selection in sync so the user can see the source way's tags
Expand Down Expand Up @@ -553,7 +548,7 @@ private static String prefKey(String subKey) {
private static class KeyboardModifiersProperty extends AbstractToStringProperty<Map<Modifier, Boolean>> {

KeyboardModifiersProperty(String key, String defaultValue) {
super(key, createFromString(defaultValue));
this(key, createFromString(defaultValue));
}

KeyboardModifiersProperty(String key, Map<Modifier, Boolean> defaultValue) {
Expand Down
7 changes: 4 additions & 3 deletions src/org/openstreetmap/josm/actions/search/SearchAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.openstreetmap.josm.gui.preferences.ToolbarPreferences.ActionParser;
import org.openstreetmap.josm.gui.progress.ProgressMonitor;
import org.openstreetmap.josm.gui.tagging.ac.AutoCompComboBoxModel;
import org.openstreetmap.josm.gui.widgets.JosmComboBoxModel;
import org.openstreetmap.josm.spi.preferences.Config;
import org.openstreetmap.josm.tools.Logging;
import org.openstreetmap.josm.tools.Shortcut;
Expand All @@ -65,10 +66,10 @@ public class SearchAction extends JosmAction implements ParameterizedAction {

private static final String SEARCH_EXPRESSION = "searchExpression";

private static AutoCompComboBoxModel<SearchSetting> model = new AutoCompComboBoxModel<>();
private static final AutoCompComboBoxModel<SearchSetting> model = new AutoCompComboBoxModel<>();

/** preferences reader/writer with automatic transmogrification to and from String */
private static AutoCompComboBoxModel<SearchSetting>.Preferences prefs = model.prefs(
private static final JosmComboBoxModel<SearchSetting>.Preferences prefs = model.prefs(
SearchSetting::readFromString, SearchSetting::writeToString);

static {
Expand Down Expand Up @@ -490,6 +491,6 @@ protected void updateEnabledState() {

@Override
public List<ActionParameter<?>> getActionParameters() {
return Collections.<ActionParameter<?>>singletonList(new SearchSettingsActionParameter(SEARCH_EXPRESSION));
return Collections.singletonList(new SearchSettingsActionParameter(SEARCH_EXPRESSION));
}
}
6 changes: 3 additions & 3 deletions src/org/openstreetmap/josm/command/PurgeCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public PurgeCommand(DataSet data, Collection<OsmPrimitive> toPurge, Collection<O
}

private void init(Collection<OsmPrimitive> toPurge, Collection<OsmPrimitive> makeIncomplete) {
/**
/*
* The topological sort is to avoid missing way nodes and missing
* relation members when adding primitives back to the dataset on undo.
*
Expand Down Expand Up @@ -159,7 +159,7 @@ public static List<OsmPrimitive> topoSort(Collection<OsmPrimitive> sel) {
// Nodes not deleted in the first pass
Set<OsmPrimitive> remainingNodes = new HashSet<>(in.size());

/**
/*
* First add nodes that have no way referrer.
*/
outer:
Expand All @@ -179,7 +179,7 @@ public static List<OsmPrimitive> topoSort(Collection<OsmPrimitive> sel) {
}
}

/**
/*
* Then add all ways, each preceded by its (remaining) nodes.
*/
for (Iterator<OsmPrimitive> it = in.iterator(); it.hasNext();) {
Expand Down
5 changes: 2 additions & 3 deletions src/org/openstreetmap/josm/data/Bounds.java
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,8 @@ public double getArea() {
* @return The string encoded bounds
*/
public String encodeAsString(String separator) {
return new StringBuilder()
.append(minLat).append(separator).append(minLon).append(separator)
.append(maxLat).append(separator).append(maxLon).toString();
return minLat + separator + minLon + separator +
maxLat + separator + maxLon;
}

/**
Expand Down
Loading

0 comments on commit 48f9f83

Please sign in to comment.