Index: /trunk/src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java (revision 18846) +++ /trunk/src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java (revision 18847) @@ -27,4 +27,7 @@ import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.openstreetmap.josm.data.Bounds; @@ -127,4 +130,7 @@ } + /** + * The core factory for "simple" {@link Match} objects + */ public static class CoreSimpleMatchFactory implements SimpleMatchFactory { private final Collection keywords = Arrays.asList("id", "version", "type", "user", "role", @@ -156,55 +162,61 @@ default: if (tokenizer != null) { - switch (keyword) { - case "id": - return new Id(tokenizer); - case "version": - return new Version(tokenizer); - case "type": - return new ExactType(tokenizer.readTextOrNumber()); - case "preset": - return new Preset(tokenizer.readTextOrNumber()); - case "user": - return new UserMatch(tokenizer.readTextOrNumber()); - case "role": - return new RoleMatch(tokenizer.readTextOrNumber()); - case "changeset": - return new ChangesetId(tokenizer); - case "nodes": - return new NodeCountRange(tokenizer); - case "ways": - return new WayCountRange(tokenizer); - case "members": - return new MemberCountRange(tokenizer); - case "tags": - return new TagCountRange(tokenizer); - case "areasize": - return new AreaSize(tokenizer); - case "waylength": - return new WayLength(tokenizer); - case "nth": - return new Nth(tokenizer, false); - case "nth%": - return new Nth(tokenizer, true); - case "hasRole": - return new HasRole(tokenizer); - case "timestamp": - // add leading/trailing space in order to get expected split (e.g. "a--" => {"a", ""}) - String rangeS = ' ' + tokenizer.readTextOrNumber() + ' '; - String[] rangeA = rangeS.split("/", -1); - if (rangeA.length == 1) { - return new KeyValue(keyword, rangeS.trim(), regexSearch, caseSensitive); - } else if (rangeA.length == 2) { - return TimestampRange.create(rangeA); - } else { - throw new SearchParseError("" + tr("Expecting {0} after {1}", "min/max", "timestamp") - + ""); - } - } + return getTokenizer(keyword, caseSensitive, regexSearch, tokenizer); } else { throw new SearchParseError("" + tr("Expecting {0} after {1}", ":", "" + keyword + "") + ""); } } - throw new IllegalStateException("Not expecting keyword " + keyword); + } + + private static Match getTokenizer(String keyword, boolean caseSensitive, boolean regexSearch, PushbackTokenizer tokenizer) + throws SearchParseError { + switch (keyword) { + case "id": + return new Id(tokenizer); + case "version": + return new Version(tokenizer); + case "type": + return new ExactType(tokenizer.readTextOrNumber()); + case "preset": + return new Preset(tokenizer.readTextOrNumber()); + case "user": + return new UserMatch(tokenizer.readTextOrNumber()); + case "role": + return new RoleMatch(tokenizer.readTextOrNumber()); + case "changeset": + return new ChangesetId(tokenizer); + case "nodes": + return new NodeCountRange(tokenizer); + case "ways": + return new WayCountRange(tokenizer); + case "members": + return new MemberCountRange(tokenizer); + case "tags": + return new TagCountRange(tokenizer); + case "areasize": + return new AreaSize(tokenizer); + case "waylength": + return new WayLength(tokenizer); + case "nth": + return new Nth(tokenizer, false); + case "nth%": + return new Nth(tokenizer, true); + case "hasRole": + return new HasRole(tokenizer); + case "timestamp": + // add leading/trailing space in order to get expected split (e.g. "a--" => {"a", ""}) + String rangeS = ' ' + tokenizer.readTextOrNumber() + ' '; + String[] rangeA = rangeS.split("/", -1); + if (rangeA.length == 1) { + return new KeyValue(keyword, rangeS.trim(), regexSearch, caseSensitive); + } else if (rangeA.length == 2) { + return TimestampRange.create(rangeA); + } else { + throw new SearchParseError("" + tr("Expecting {0} after {1}", "min/max", "timestamp") + + ""); + } + default: + throw new IllegalStateException("Not expecting keyword " + keyword); + } } @@ -215,4 +227,7 @@ } + /** + * The core {@link UnaryMatch} factory + */ public static class CoreUnaryMatchFactory implements UnaryMatchFactory { private static final Collection keywords = Arrays.asList("parent", "child"); @@ -242,13 +257,48 @@ } + /** + * A factory for getting {@link Match} objects + */ public interface SimpleMatchFactory extends MatchFactory { + /** + * Get the {@link Match} object + * @param keyword The keyword to get/create the correct {@link Match} object + * @param caseSensitive {@code true} if the search is case-sensitive + * @param regexSearch {@code true} if the search is regex-based + * @param tokenizer May be used to construct the {@link Match} object + * @return The {@link Match} object for the keyword and its arguments + * @throws SearchParseError If the {@link Match} object could not be constructed. + */ Match get(String keyword, boolean caseSensitive, boolean regexSearch, PushbackTokenizer tokenizer) throws SearchParseError; } + /** + * A factory for getting {@link UnaryMatch} objects + */ public interface UnaryMatchFactory extends MatchFactory { + /** + * Get the {@link UnaryMatch} object + * @param keyword The keyword to get/create the correct {@link UnaryMatch} object + * @param matchOperand May be used to construct the {@link UnaryMatch} object + * @param tokenizer May be used to construct the {@link UnaryMatch} object + * @return The {@link UnaryMatch} object for the keyword and its arguments + * @throws SearchParseError If the {@link UnaryMatch} object could not be constructed. + */ UnaryMatch get(String keyword, Match matchOperand, PushbackTokenizer tokenizer) throws SearchParseError; } + /** + * A factor for getting {@link AbstractBinaryMatch} objects + */ public interface BinaryMatchFactory extends MatchFactory { + /** + * Get the {@link AbstractBinaryMatch} object + * @param keyword The keyword to get/create the correct {@link AbstractBinaryMatch} object + * @param lhs May be used to construct the {@link AbstractBinaryMatch} object (see {@link AbstractBinaryMatch#getLhs()}) + * @param rhs May be used to construct the {@link AbstractBinaryMatch} object (see {@link AbstractBinaryMatch#getRhs()}) + * @param tokenizer May be used to construct the {@link AbstractBinaryMatch} object + * @return The {@link AbstractBinaryMatch} object for the keyword and its arguments + * @throws SearchParseError If the {@link AbstractBinaryMatch} object could not be constructed. + */ AbstractBinaryMatch get(String keyword, Match lhs, Match rhs, PushbackTokenizer tokenizer) throws SearchParseError; } @@ -303,4 +353,7 @@ } + /** + * A common subclass of {@link Match} for matching against tags + */ public abstract static class TaggedMatch extends Match { @@ -330,8 +383,8 @@ */ public abstract static class UnaryMatch extends Match { - + @Nonnull protected final Match match; - protected UnaryMatch(Match match) { + protected UnaryMatch(@Nullable Match match) { if (match == null) { // "operator" (null) should mean the same as "operator()" @@ -349,5 +402,5 @@ @Override public int hashCode() { - return 31 + ((match == null) ? 0 : match.hashCode()); + return 31 + match.hashCode(); } @@ -359,10 +412,5 @@ return false; UnaryMatch other = (UnaryMatch) obj; - if (match == null) { - if (other.match != null) - return false; - } else if (!match.equals(other.match)) - return false; - return true; + return match.equals(other.match); } } @@ -430,15 +478,5 @@ return false; AbstractBinaryMatch other = (AbstractBinaryMatch) obj; - if (lhs == null) { - if (other.lhs != null) - return false; - } else if (!lhs.equals(other.lhs)) - return false; - if (rhs == null) { - if (other.rhs != null) - return false; - } else if (!rhs.equals(other.rhs)) - return false; - return true; + return Objects.equals(lhs, other.lhs) && Objects.equals(rhs, other.rhs); } } @@ -536,10 +574,5 @@ if (defaultValue != other.defaultValue) return false; - if (key == null) { - if (other.key != null) - return false; - } else if (!key.equals(other.key)) - return false; - return true; + return Objects.equals(key, other.key); } } @@ -794,4 +827,12 @@ } + /** + * Match a primitive based off of a value comparison. This currently supports: + * + */ public static class ValueComparison extends TaggedMatch { private final String key; @@ -801,4 +842,11 @@ private static final Pattern ISO8601 = Pattern.compile("\\d+-\\d+-\\d+"); + /** + * Create a new {@link ValueComparison} object + * @param key The key to get the value from + * @param referenceValue The value to compare to + * @param compareMode The compare mode to use; {@code < 0} is {@code currentValue < referenceValue} and + * {@code > 0} is {@code currentValue > referenceValue}. {@code 0} is effectively an equality check. + */ public ValueComparison(String key, String referenceValue, int compareMode) { this.key = key; @@ -809,6 +857,6 @@ v = Double.valueOf(referenceValue); } - } catch (NumberFormatException ignore) { - Logging.trace(ignore); + } catch (NumberFormatException numberFormatException) { + Logging.trace(numberFormatException); } this.referenceNumber = v; @@ -855,20 +903,7 @@ if (compareMode != other.compareMode) return false; - if (key == null) { - if (other.key != null) - return false; - } else if (!key.equals(other.key)) - return false; - if (referenceNumber == null) { - if (other.referenceNumber != null) - return false; - } else if (!referenceNumber.equals(other.referenceNumber)) - return false; - if (referenceValue == null) { - if (other.referenceValue != null) - return false; - } else if (!referenceValue.equals(other.referenceValue)) - return false; - return true; + return Objects.equals(key, other.key) + && Objects.equals(referenceNumber, other.referenceNumber) + && Objects.equals(referenceValue, other.referenceValue); } @@ -895,7 +930,28 @@ public static class ExactKeyValue extends TaggedMatch { + /** + * The mode to use for the comparison + */ public enum Mode { - ANY, ANY_KEY, ANY_VALUE, EXACT, NONE, MISSING_KEY, - ANY_KEY_REGEXP, ANY_VALUE_REGEXP, EXACT_REGEXP, MISSING_KEY_REGEXP; + /** Matches everything */ + ANY, + /** Any key with the specified value will match */ + ANY_KEY, + /** Any value with the specified key will match */ + ANY_VALUE, + /** A key with the specified value will match */ + EXACT, + /** Nothing matches */ + NONE, + /** The key does not exist */ + MISSING_KEY, + /** Similar to {@link #ANY_KEY}, but the value matches a regex */ + ANY_KEY_REGEXP, + /** Similar to {@link #ANY_VALUE}, but the key matches a regex */ + ANY_VALUE_REGEXP, + /** Both the key and the value matches their respective regex */ + EXACT_REGEXP, + /** No key matching the regex exists */ + MISSING_KEY_REGEXP } @@ -971,9 +1027,9 @@ return false; case MISSING_KEY: - return !osm.hasTag(key); + return !osm.hasKey(key); case ANY: return true; case ANY_VALUE: - return osm.hasTag(key); + return osm.hasKey(key); case ANY_KEY: return osm.getKeys().values().stream().anyMatch(v -> v.equals(value)); @@ -1021,27 +1077,10 @@ return false; ExactKeyValue other = (ExactKeyValue) obj; - if (key == null) { - if (other.key != null) - return false; - } else if (!key.equals(other.key)) - return false; - if (keyPattern == null) { - if (other.keyPattern != null) - return false; - } else if (!keyPattern.equals(other.keyPattern)) - return false; if (mode != other.mode) return false; - if (value == null) { - if (other.value != null) - return false; - } else if (!value.equals(other.value)) - return false; - if (valuePattern == null) { - if (other.valuePattern != null) - return false; - } else if (!valuePattern.equals(other.valuePattern)) - return false; - return true; + return Objects.equals(key, other.key) + && Objects.equals(value, other.value) + && Objects.equals(keyPattern, other.keyPattern) + && Objects.equals(valuePattern, other.valuePattern); } } @@ -1124,18 +1163,12 @@ if (caseSensitive != other.caseSensitive) return false; - if (search == null) { - if (other.search != null) - return false; - } else if (!search.equals(other.search)) - return false; - if (searchRegex == null) { - if (other.searchRegex != null) - return false; - } else if (!searchRegex.equals(other.searchRegex)) - return false; - return true; - } - } - + return Objects.equals(search, other.search) + && Objects.equals(searchRegex, other.searchRegex); + } + } + + /** + * Filter OsmPrimitives based off of the base primitive type + */ public static class ExactType extends Match { private final OsmPrimitiveType type; @@ -1220,10 +1253,5 @@ return false; UserMatch other = (UserMatch) obj; - if (user == null) { - if (other.user != null) - return false; - } else if (!user.equals(other.user)) - return false; - return true; + return Objects.equals(user, other.user); } } @@ -1233,4 +1261,5 @@ */ private static class RoleMatch extends Match { + @Nonnull private final String role; @@ -1259,5 +1288,5 @@ @Override public int hashCode() { - return 31 + ((role == null) ? 0 : role.hashCode()); + return 31 + role.hashCode(); } @@ -1269,10 +1298,5 @@ return false; RoleMatch other = (RoleMatch) obj; - if (role == null) { - if (other.role != null) - return false; - } else if (!role.equals(other.role)) - return false; - return true; + return role.equals(other.role); } } @@ -1283,5 +1307,5 @@ private static class Nth extends Match { - private final int nth; + private final int nthObject; private final boolean modulo; @@ -1291,7 +1315,7 @@ private Nth(int nth, boolean modulo) throws SearchParseError { - this.nth = nth; + this.nthObject = nth; this.modulo = modulo; - if (this.modulo && this.nth == 0) { + if (this.modulo && this.nthObject == 0) { throw new SearchParseError(tr("Non-zero integer expected")); } @@ -1314,7 +1338,7 @@ continue; } - if (nth < 0 && idx - maxIndex == nth) { + if (nthObject < 0 && idx - maxIndex == nthObject) { return true; - } else if (idx == nth || (modulo && idx % nth == 0)) + } else if (idx == nthObject || (modulo && idx % nthObject == 0)) return true; } @@ -1324,10 +1348,10 @@ @Override public String toString() { - return "Nth{nth=" + nth + ", modulo=" + modulo + '}'; + return "Nth{nth=" + nthObject + ", modulo=" + modulo + '}'; } @Override public int hashCode() { - return Objects.hash(modulo, nth); + return Objects.hash(modulo, nthObject); } @@ -1340,5 +1364,5 @@ Nth other = (Nth) obj; return modulo == other.modulo - && nth == other.nth; + && nthObject == other.nthObject; } } @@ -1521,5 +1545,5 @@ final long maxDate; try { - // if min timestamp is empty: use lowest possible date + // if min timestamp is empty: use the lowest possible date minDate = DateUtils.parseInstant(rangeA1.isEmpty() ? "1980" : rangeA1).toEpochMilli(); } catch (UncheckedParseException | DateTimeException ex) { @@ -1573,10 +1597,5 @@ return false; HasRole other = (HasRole) obj; - if (role == null) { - if (other.role != null) - return false; - } else if (!role.equals(other.role)) - return false; - return true; + return Objects.equals(role, other.role); } } @@ -1644,5 +1663,5 @@ /** * Match objects that are incomplete, where only id and type are known. - * Typically some members of a relation are incomplete until they are + * Typically, some members of a relation are incomplete until they are * fetched from the server. */ @@ -1865,5 +1884,5 @@ /** * Matches objects which are not outside the source area ("downloaded area"). - * Unlike {@link InDataSourceArea} this matches also if no source area is set (e.g., for new layers). + * Unlike {@link InDataSourceArea}, this matches also if no source area is set (e.g., for new layers). */ public static class NotOutsideDataSourceArea extends InDataSourceArea { @@ -1955,10 +1974,5 @@ return false; Preset other = (Preset) obj; - if (presets == null) { - if (other.presets != null) - return false; - } else if (!presets.equals(other.presets)) - return false; - return true; + return Objects.equals(presets, other.presets); } } @@ -2156,5 +2170,8 @@ // key:value form where value is a string (may be OSM key search) final String value = tokenizer.readTextOrNumber(); - return new KeyValue(key, value != null ? value : "", regexSearch, caseSensitive).validate(); + if (value == null) { + return new ExactKeyValue(regexSearch, caseSensitive, key, "*").validate(); + } + return new KeyValue(key, value, regexSearch, caseSensitive).validate(); } else if (tokenizer.readIfEqual(Token.QUESTION_MARK)) return new BooleanMatch(key, false); Index: /trunk/src/org/openstreetmap/josm/tools/SearchCompilerQueryWizard.java =================================================================== --- /trunk/src/org/openstreetmap/josm/tools/SearchCompilerQueryWizard.java (revision 18846) +++ /trunk/src/org/openstreetmap/josm/tools/SearchCompilerQueryWizard.java (revision 18847) @@ -38,5 +38,6 @@ public static String constructQuery(final String search) { try { - Matcher matcher = Pattern.compile("\\s+GLOBAL\\s*$", Pattern.CASE_INSENSITIVE).matcher(search); + Matcher matcher = Pattern.compile("\\s+GLOBAL\\s*$", Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS) + .matcher(search); if (matcher.find()) { final Match match = SearchCompiler.compile(matcher.replaceFirst("")); @@ -44,5 +45,6 @@ } - matcher = Pattern.compile("\\s+IN BBOX\\s*$", Pattern.CASE_INSENSITIVE).matcher(search); + matcher = Pattern.compile("\\s+IN BBOX\\s*$", Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS) + .matcher(search); if (matcher.find()) { final Match match = SearchCompiler.compile(matcher.replaceFirst("")); @@ -50,5 +52,6 @@ } - matcher = Pattern.compile("\\s+(?IN|AROUND)\\s+(?[^\" ]+|\"[^\"]+\")\\s*$", Pattern.CASE_INSENSITIVE).matcher(search); + matcher = Pattern.compile("\\s+(?IN|AROUND)\\s+(?[^\" ]+|\"[^\"]+\")\\s*$", + Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CHARACTER_CLASS).matcher(search); if (matcher.find()) { final Match match = SearchCompiler.compile(matcher.replaceFirst("")); @@ -79,8 +82,9 @@ final EnumSet types = EnumSet.noneOf(OsmPrimitiveType.class); final String query = constructQuery(conjunction, types); - (types.isEmpty() || types.size() == 3 + queryLines.addAll((types.isEmpty() || types.size() == 3 ? Stream.of("nwr") : types.stream().map(OsmPrimitiveType::getAPIName)) - .forEach(type -> queryLines.add(" " + type + query + queryLineSuffix + ";")); + .map(type -> " " + type + query + queryLineSuffix + ";") + .collect(Collectors.toList())); } queryLines.add(");"); Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java (revision 18846) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java (revision 18847) @@ -22,4 +22,5 @@ import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.openstreetmap.josm.TestUtils; @@ -45,5 +46,4 @@ import org.openstreetmap.josm.gui.tagging.presets.items.Key; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; -import org.openstreetmap.josm.tools.Logging; import nl.jqno.equalsverifier.EqualsVerifier; @@ -745,19 +745,22 @@ } + static Set> testEqualsContract() { + TestUtils.assumeWorkingEqualsVerifier(); + final Set> matchers = TestUtils.getJosmSubtypes(Match.class); + assertTrue(matchers.size() >= 10, "There should be at least 10 matchers from JOSM core"); + return matchers; + } + /** * Unit test of methods {@link Match#equals} and {@link Match#hashCode}, including all subclasses. - */ - @Test - void testEqualsContract() { - TestUtils.assumeWorkingEqualsVerifier(); - Set> matchers = TestUtils.getJosmSubtypes(Match.class); - assertTrue(matchers.size() >= 10); // if it finds less than 10 classes, something is broken - for (Class c : matchers) { - Logging.debug(c.toString()); - EqualsVerifier.forClass(c).usingGetClass() - .suppress(Warning.NONFINAL_FIELDS, Warning.INHERITED_DIRECTLY_FROM_OBJECT) - .withPrefabValues(TaggingPreset.class, newTaggingPreset("foo"), newTaggingPreset("bar")) - .verify(); - } + * @param clazz The class to test + */ + @ParameterizedTest + @MethodSource + void testEqualsContract(Class clazz) { + EqualsVerifier.forClass(clazz).usingGetClass() + .suppress(Warning.NONFINAL_FIELDS, Warning.INHERITED_DIRECTLY_FROM_OBJECT) + .withPrefabValues(TaggingPreset.class, newTaggingPreset("foo"), newTaggingPreset("bar")) + .verify(); } Index: /trunk/test/unit/org/openstreetmap/josm/tools/SearchCompilerQueryWizardTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/tools/SearchCompilerQueryWizardTest.java (revision 18846) +++ /trunk/test/unit/org/openstreetmap/josm/tools/SearchCompilerQueryWizardTest.java (revision 18847) @@ -7,20 +7,11 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; -import org.openstreetmap.josm.testutils.JOSMTestRules; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.openstreetmap.josm.testutils.annotations.I18n; /** * Unit tests of {@link SearchCompilerQueryWizard} class. */ +@I18n("de") class SearchCompilerQueryWizardTest { - /** - * Base test environment is enough - */ - @RegisterExtension - @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules().i18n("de"); - private static String constructQuery(String s) { return SearchCompilerQueryWizard.constructQuery(s); @@ -264,3 +255,12 @@ "type:node AND (*=forward OR *=backward)"); } + + /** + * Test for ticket