Changeset 14705 in josm for trunk


Ignore:
Timestamp:
2019-01-19T18:09:55+01:00 (5 years ago)
Author:
GerdP
Message:

fix some sonar issues

  • split complex methods into smaller blocks
  • reducee visibility of inner classes only used by TagChecker
  • reduce memory footprint: use simple HashSet instead of MultiMap for additionalPresetsValueData
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java

    r14704 r14705  
    7474    private static final Map<String, String> harmonizedKeys = new HashMap<>();
    7575    /** The spell check preset values which are not stored in TaggingPresets */
    76     private static volatile MultiMap<String, String> additionalPresetsValueData;
    77     /** The spell check preset values which are not stored in TaggingPresets */
    78     private static volatile MultiMap<String, String> oftenUsedValueData = new MultiMap<>();
     76    private static volatile HashSet<String> additionalPresetsValueData;
     77    /** often used tags which are not in presets */
     78    private static volatile MultiMap<String, String> oftenUsedTags = new MultiMap<>();
    7979
    8080    /** The TagChecker data */
    … …  
    113113    public static final String PREF_SOURCES = PREFIX + ".source";
    114114
     115    private static final String BEFORE_UPLOAD = "BeforeUpload";
    115116    /**
    116117     * The preference key to check keys - used before upload
    117118     */
    118     public static final String PREF_CHECK_KEYS_BEFORE_UPLOAD = PREF_CHECK_KEYS + "BeforeUpload";
     119    public static final String PREF_CHECK_KEYS_BEFORE_UPLOAD = PREF_CHECK_KEYS + BEFORE_UPLOAD;
    119120    /**
    120121     * The preference key to check values - used before upload
    121122     */
    122     public static final String PREF_CHECK_VALUES_BEFORE_UPLOAD = PREF_CHECK_VALUES + "BeforeUpload";
     123    public static final String PREF_CHECK_VALUES_BEFORE_UPLOAD = PREF_CHECK_VALUES + BEFORE_UPLOAD;
    123124    /**
    124125     * The preference key to run complex tests - used before upload
    125126     */
    126     public static final String PREF_CHECK_COMPLEX_BEFORE_UPLOAD = PREF_CHECK_COMPLEX + "BeforeUpload";
     127    public static final String PREF_CHECK_COMPLEX_BEFORE_UPLOAD = PREF_CHECK_COMPLEX + BEFORE_UPLOAD;
    127128    /**
    128129     * The preference key to search for fixmes - used before upload
    129130     */
    130     public static final String PREF_CHECK_FIXMES_BEFORE_UPLOAD = PREF_CHECK_FIXMES + "BeforeUpload";
     131    public static final String PREF_CHECK_FIXMES_BEFORE_UPLOAD = PREF_CHECK_FIXMES + BEFORE_UPLOAD;
    131132
    132133    private static final int MAX_LEVENSHTEIN_DISTANCE = 2;
    … …  
    166167    protected static final int MISSPELLED_VALUE_NO_FIX  = 1215;
    167168    // CHECKSTYLE.ON: SingleSpaceSeparator
    168     // 1250 and up is used by tagcheck
     169    // 1250 and up is used by TagCheckLevel in class CheckerData
    169170
    170171    protected EditableList sourcesList;
    … …  
    210211
    211212    /**
    212      * Reads the spellcheck file into a HashMap.
     213     * Reads the spell-check file into a HashMap.
    213214     * The data file is a list of words, beginning with +/-. If it starts with +,
    214215     * the word is valid, but if it starts with -, the word should be replaced
    … …  
    225226        harmonizedKeys.clear();
    226227        ignoreForLevenshtein.clear();
     228        oftenUsedTags.clear();
    227229
    228230        StringBuilder errorSources = new StringBuilder();
    … …  
    252254                        }
    253255                    } else if (ignorefile) {
    254                         line = line.trim();
    255                         if (line.length() < 4) {
    256                             continue;
    257                         }
    258                         try {
    259                             String key = line.substring(0, 2);
    260                             line = line.substring(2);
    261 
    262                             switch (key) {
    263                             case "S:":
    264                                 ignoreDataStartsWith.add(line);
    265                                 break;
    266                             case "E:":
    267                                 ignoreDataEquals.add(line);
    268                                 break;
    269                             case "F:":
    270                                 ignoreDataEndsWith.add(line);
    271                                 break;
    272                             case "K:":
    273                                 Tag tag = Tag.ofString(line);
    274                                 ignoreDataTag.add(tag);
    275                                 oftenUsedValueData.put(tag.getKey(), tag.getValue());
    276                                 break;
    277                             default:
    278                                 if (!key.startsWith(";")) {
    279                                     Logging.warn("Unsupported TagChecker key: " + key);
    280                                 }
    281                             }
    282                         } catch (IllegalArgumentException e) {
    283                             Logging.error("Invalid line in {0} : {1}", source, e.getMessage());
    284                             Logging.trace(e);
    285                         }
     256                        parseIgnoreFileLine(source, line);
    286257                    } else if (tagcheckerfile) {
    287258                        if (!line.isEmpty()) {
    … …  
    299270                    } else if (line.charAt(0) == '-' && okValue != null) {
    300271                        String hk = harmonizeKey(line.substring(1));
    301                         if (!okValue.equals(hk)) {
    302                             if (harmonizedKeys.put(hk, okValue) != null) {
    303                                 Logging.debug(tr("Line was ignored: {0}", line));
    304                             }
     272                        if (!okValue.equals(hk) && harmonizedKeys.put(hk, okValue) != null) {
     273                            Logging.debug(tr("Line was ignored: {0}", line));
    305274                        }
    306275                    } else {
    … …  
    325294
    326295    /**
     296     * Parse a line found in a configuration file
     297     * @param source name of configuration file
     298     * @param line the line to parse
     299     */
     300    private static void parseIgnoreFileLine(String source, String line) {
     301        line = line.trim();
     302        if (line.length() < 4) {
     303            return;
     304        }
     305        try {
     306            String key = line.substring(0, 2);
     307            line = line.substring(2);
     308
     309            switch (key) {
     310            case "S:":
     311                ignoreDataStartsWith.add(line);
     312                break;
     313            case "E:":
     314                ignoreDataEquals.add(line);
     315                break;
     316            case "F:":
     317                ignoreDataEndsWith.add(line);
     318                break;
     319            case "K:":
     320                Tag tag = Tag.ofString(line);
     321                ignoreDataTag.add(tag);
     322                oftenUsedTags.put(tag.getKey(), tag.getValue());
     323                break;
     324            default:
     325                if (!key.startsWith(";")) {
     326                    Logging.warn("Unsupported TagChecker key: " + key);
     327                }
     328            }
     329        } catch (IllegalArgumentException e) {
     330            Logging.error("Invalid line in {0} : {1}", source, e.getMessage());
     331            Logging.trace(e);
     332        }
     333    }
     334
     335    /**
    327336     * Reads the presets data.
    328337     *
    … …  
    335344        Collection<TaggingPreset> presets = TaggingPresets.getTaggingPresets();
    336345        if (!presets.isEmpty()) {
    337             additionalPresetsValueData = new MultiMap<>();
    338             for (String a : AbstractPrimitive.getUninterestingKeys()) {
    339                 additionalPresetsValueData.putVoid(a);
    340             }
    341             // TODO directionKeys are no longer in OsmPrimitive (search pattern is used instead)
    342             for (String a : Config.getPref().getList(ValidatorPrefHelper.PREFIX + ".knownkeys",
    343                     Arrays.asList("is_in", "int_ref", "fixme", "population"))) {
    344                 additionalPresetsValueData.putVoid(a);
    345             }
     346            initAdditionalPresetsValueData();
    346347            for (TaggingPreset p : presets) {
    347348                for (TaggingPresetItem i : p.data) {
    … …  
    358359    }
    359360
     361    private static void initAdditionalPresetsValueData() {
     362        additionalPresetsValueData = new HashSet<>();
     363        for (String a : AbstractPrimitive.getUninterestingKeys()) {
     364            additionalPresetsValueData.add(a);
     365        }
     366        for (String a : Config.getPref().getList(ValidatorPrefHelper.PREFIX + ".knownkeys",
     367                Arrays.asList("is_in", "int_ref", "fixme", "population"))) {
     368            additionalPresetsValueData.add(a);
     369        }
     370    }
     371
    360372    private static void addPresetValue(KeyedItem ky) {
    361373        if (ky.key != null && ky.getValues() != null) {
    … …  
    386398        if (res != null)
    387399            return res;
    388         return additionalPresetsValueData.get(key);
     400        if (additionalPresetsValueData.contains(key))
     401            return Collections.emptySet();
     402        return null;
    389403    }
    390404
    … …  
    494508            String key = prop.getKey();
    495509            String value = prop.getValue();
    496             if (checkValues && (containsLow(value)) && !withErrors.contains(p, "ICV")) {
    497                 errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_VALUE)
    498                         .message(tr("Tag value contains character with code less than 0x20"), s, key)
    499                         .primitives(p)
    500                         .build());
    501                 withErrors.put(p, "ICV");
    502             }
    503             if (checkKeys && (containsLow(key)) && !withErrors.contains(p, "ICK")) {
    504                 errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_KEY)
    505                         .message(tr("Tag key contains character with code less than 0x20"), s, key)
    506                         .primitives(p)
    507                         .build());
    508                 withErrors.put(p, "ICK");
    509             }
    510             if (checkValues && (value != null && value.length() > Tagged.MAX_TAG_LENGTH) && !withErrors.contains(p, "LV")) {
    511                 errors.add(TestError.builder(this, Severity.ERROR, LONG_VALUE)
    512                         .message(tr("Tag value longer than {0} characters ({1} characters)", Tagged.MAX_TAG_LENGTH, value.length()), s, key)
    513                         .primitives(p)
    514                         .build());
    515                 withErrors.put(p, "LV");
    516             }
    517             if (checkKeys && (key != null && key.length() > Tagged.MAX_TAG_LENGTH) && !withErrors.contains(p, "LK")) {
    518                 errors.add(TestError.builder(this, Severity.ERROR, LONG_KEY)
    519                         .message(tr("Tag key longer than {0} characters ({1} characters)", Tagged.MAX_TAG_LENGTH, key.length()), s, key)
    520                         .primitives(p)
    521                         .build());
    522                 withErrors.put(p, "LK");
    523             }
    524             if (checkValues && (value == null || value.trim().isEmpty()) && !withErrors.contains(p, "EV")) {
    525                 errors.add(TestError.builder(this, Severity.WARNING, EMPTY_VALUES)
    526                         .message(tr("Tags with empty values"), s, key)
    527                         .primitives(p)
    528                         .build());
    529                 withErrors.put(p, "EV");
    530             }
    531             if (checkKeys && key != null && key.indexOf(' ') >= 0 && !withErrors.contains(p, "IPK")) {
    532                 errors.add(TestError.builder(this, Severity.WARNING, INVALID_KEY_SPACE)
    533                         .message(tr("Invalid white space in property key"), s, key)
    534                         .primitives(p)
    535                         .build());
    536                 withErrors.put(p, "IPK");
    537             }
    538             if (checkValues && value != null && (value.startsWith(" ") || value.endsWith(" ")) && !withErrors.contains(p, "SPACE")) {
    539                 errors.add(TestError.builder(this, Severity.WARNING, INVALID_SPACE)
    540                         .message(tr("Property values start or end with white space"), s, key)
    541                         .primitives(p)
    542                         .build());
    543                 withErrors.put(p, "SPACE");
    544             }
    545             if (checkValues && value != null && value.contains("  ") && !withErrors.contains(p, "SPACE")) {
    546                 errors.add(TestError.builder(this, Severity.WARNING, MULTIPLE_SPACES)
    547                         .message(tr("Property values contain multiple white spaces"), s, key)
    548                         .primitives(p)
    549                         .build());
    550                 withErrors.put(p, "SPACE");
    551             }
    552             if (checkValues && value != null && !value.equals(Entities.unescape(value)) && !withErrors.contains(p, "HTML")) {
    553                 errors.add(TestError.builder(this, Severity.OTHER, INVALID_HTML)
    554                         .message(tr("Property values contain HTML entity"), s, key)
    555                         .primitives(p)
    556                         .build());
    557                 withErrors.put(p, "HTML");
    558             }
    559             if (checkValues && key != null && value != null && !value.isEmpty() && additionalPresetsValueData != null
    560                     && !isTagIgnored(key, value)) {
    561                 if (!isKeyInPresets(key)) {
    562                     String prettifiedKey = harmonizeKey(key);
    563                     String fixedKey = isKeyInPresets(prettifiedKey) ? prettifiedKey : harmonizedKeys.get(prettifiedKey);
    564                     if (fixedKey != null && !"".equals(fixedKey) && !fixedKey.equals(key)) {
    565                         // misspelled preset key
    566                         final TestError.Builder error = TestError.builder(this, Severity.WARNING, MISSPELLED_KEY)
    567                                 .message(tr("Misspelled property key"), marktr("Key ''{0}'' looks like ''{1}''."), key, fixedKey)
    568                                 .primitives(p);
    569                         if (p.hasKey(fixedKey)) {
    570                             errors.add(error.build());
    571                         } else {
    572                             errors.add(error.fix(() -> new ChangePropertyKeyCommand(p, key, fixedKey)).build());
    573                         }
    574                         withErrors.put(p, "WPK");
    575                     } else {
    576                         errors.add(TestError.builder(this, Severity.OTHER, INVALID_VALUE)
    577                                 .message(tr("Presets do not contain property key"), marktr("Key ''{0}'' not in presets."), key)
    578                                 .primitives(p)
    579                                 .build());
    580                         withErrors.put(p, "UPK");
    581                     }
    582                 } else if (!isTagInPresets(key, value)) {
    583                     tryGuess(p, key, value, withErrors);
    584                 }
     510
     511            if (checkKeys) {
     512                checkSingleTagKeySimple(withErrors, p, s, key);
     513            }
     514            if (checkValues) {
     515                checkSingleTagValueSimple(withErrors, p, s, key, value);
     516                checkSingleTagComplex(withErrors, p, key, value);
    585517            }
    586518            if (checkFixmes && key != null && value != null && !value.isEmpty() && isFixme(key, value) && !withErrors.contains(p, "FIXME")) {
    … …  
    594526    }
    595527
     528    private void checkSingleTagValueSimple(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String s, String key, String value) {
     529        if (!checkValues || value == null)
     530            return;
     531        if ((containsLow(value)) && !withErrors.contains(p, "ICV")) {
     532            errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_VALUE)
     533                    .message(tr("Tag value contains character with code less than 0x20"), s, key)
     534                    .primitives(p)
     535                    .build());
     536            withErrors.put(p, "ICV");
     537        }
     538        if ((value.length() > Tagged.MAX_TAG_LENGTH) && !withErrors.contains(p, "LV")) {
     539            errors.add(TestError.builder(this, Severity.ERROR, LONG_VALUE)
     540                    .message(tr("Tag value longer than {0} characters ({1} characters)", Tagged.MAX_TAG_LENGTH, value.length()), s, key)
     541                    .primitives(p)
     542                    .build());
     543            withErrors.put(p, "LV");
     544        }
     545        if ((value.trim().isEmpty()) && !withErrors.contains(p, "EV")) {
     546            errors.add(TestError.builder(this, Severity.WARNING, EMPTY_VALUES)
     547                    .message(tr("Tags with empty values"), s, key)
     548                    .primitives(p)
     549                    .build());
     550            withErrors.put(p, "EV");
     551        }
     552        final String ERR_TYPE_SPACE = "SPACE";
     553        if ((value.startsWith(" ") || value.endsWith(" ")) && !withErrors.contains(p, ERR_TYPE_SPACE)) {
     554            errors.add(TestError.builder(this, Severity.WARNING, INVALID_SPACE)
     555                    .message(tr("Property values start or end with white space"), s, key)
     556                    .primitives(p)
     557                    .build());
     558            withErrors.put(p, ERR_TYPE_SPACE);
     559        }
     560        if (value.contains("  ") && !withErrors.contains(p, ERR_TYPE_SPACE)) {
     561            errors.add(TestError.builder(this, Severity.WARNING, MULTIPLE_SPACES)
     562                    .message(tr("Property values contain multiple white spaces"), s, key)
     563                    .primitives(p)
     564                    .build());
     565            withErrors.put(p, ERR_TYPE_SPACE);
     566        }
     567        if (!value.equals(Entities.unescape(value)) && !withErrors.contains(p, "HTML")) {
     568            errors.add(TestError.builder(this, Severity.OTHER, INVALID_HTML)
     569                    .message(tr("Property values contain HTML entity"), s, key)
     570                    .primitives(p)
     571                    .build());
     572            withErrors.put(p, "HTML");
     573        }
     574    }
     575
     576    private void checkSingleTagKeySimple(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String s, String key) {
     577        if (!checkKeys || key == null)
     578            return;
     579        if ((containsLow(key)) && !withErrors.contains(p, "ICK")) {
     580            errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_KEY)
     581                    .message(tr("Tag key contains character with code less than 0x20"), s, key)
     582                    .primitives(p)
     583                    .build());
     584            withErrors.put(p, "ICK");
     585        }
     586        if (key.length() > Tagged.MAX_TAG_LENGTH && !withErrors.contains(p, "LK")) {
     587            errors.add(TestError.builder(this, Severity.ERROR, LONG_KEY)
     588                    .message(tr("Tag key longer than {0} characters ({1} characters)", Tagged.MAX_TAG_LENGTH, key.length()), s, key)
     589                    .primitives(p)
     590                    .build());
     591            withErrors.put(p, "LK");
     592        }
     593        if (key.indexOf(' ') >= 0 && !withErrors.contains(p, "IPK")) {
     594            errors.add(TestError.builder(this, Severity.WARNING, INVALID_KEY_SPACE)
     595                    .message(tr("Invalid white space in property key"), s, key)
     596                    .primitives(p)
     597                    .build());
     598            withErrors.put(p, "IPK");
     599        }
     600    }
     601
     602    private void checkSingleTagComplex(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String key, String value) {
     603        if (!checkValues || key == null || value == null || value.isEmpty())
     604            return;
     605        if (additionalPresetsValueData != null && !isTagIgnored(key, value)) {
     606            if (!isKeyInPresets(key)) {
     607                spellCheckKey(withErrors, p, key);
     608            } else if (!isTagInPresets(key, value)) {
     609                if (oftenUsedTags.contains(key, value)) {
     610                    // tag is quite often used but not in presets
     611                    errors.add(TestError.builder(this, Severity.OTHER, INVALID_VALUE)
     612                            .message(tr("Presets do not contain property value"),
     613                                    marktr("Value ''{0}'' for key ''{1}'' not in presets, but is known."), value, key)
     614                            .primitives(p)
     615                            .build());
     616                    withErrors.put(p, "UPV");
     617                } else {
     618                    tryGuess(p, key, value, withErrors);
     619                }
     620            }
     621        }
     622    }
     623
     624    private void spellCheckKey(MultiMap<OsmPrimitive, String> withErrors, OsmPrimitive p, String key) {
     625        String prettifiedKey = harmonizeKey(key);
     626        String fixedKey = isKeyInPresets(prettifiedKey) ? prettifiedKey : harmonizedKeys.get(prettifiedKey);
     627        if (fixedKey != null && !"".equals(fixedKey) && !fixedKey.equals(key)) {
     628            // misspelled preset key
     629            final TestError.Builder error = TestError.builder(this, Severity.WARNING, MISSPELLED_KEY)
     630                    .message(tr("Misspelled property key"), marktr("Key ''{0}'' looks like ''{1}''."), key, fixedKey)
     631                    .primitives(p);
     632            if (p.hasKey(fixedKey)) {
     633                errors.add(error.build());
     634            } else {
     635                errors.add(error.fix(() -> new ChangePropertyKeyCommand(p, key, fixedKey)).build());
     636            }
     637            withErrors.put(p, "WPK");
     638        } else {
     639            errors.add(TestError.builder(this, Severity.OTHER, INVALID_VALUE)
     640                    .message(tr("Presets do not contain property key"), marktr("Key ''{0}'' not in presets."), key)
     641                    .primitives(p)
     642                    .build());
     643            withErrors.put(p, "UPK");
     644        }
     645    }
     646
    596647    private void tryGuess(OsmPrimitive p, String key, String value, MultiMap<OsmPrimitive, String> withErrors) {
    597648        // try to fix common typos and check again if value is still unknown
    598649        final String harmonizedValue = harmonizeValue(value);
     650        if (harmonizedValue == null || harmonizedValue.isEmpty())
     651            return;
    599652        String fixedValue = null;
    600653        Set<String> presetValues = getPresetValues(key);
    601         Set<String> oftenUsedValues = oftenUsedValueData.get(key);
     654        Set<String> oftenUsedValues = oftenUsedTags.get(key);
    602655        for (Set<String> possibleValues: Arrays.asList(presetValues, oftenUsedValues)) {
    603656            if (possibleValues != null && possibleValues.contains(harmonizedValue)) {
    … …  
    657710            }
    658711        }
    659         if (fixedValue != null) {
     712        if (fixedValue != null && !fixedValue.equals(value)) {
    660713            final String newValue = fixedValue;
    661714            // misspelled preset value
    … …  
    677730    }
    678731
    679     private boolean isNum(String harmonizedValue) {
     732    private static boolean isNum(String harmonizedValue) {
    680733        try {
    681734            Double.parseDouble(harmonizedValue);
    … …  
    879932
    880933        protected static class CheckerElement {
    881             public Object tag;
    882             public Object value;
    883             public boolean noMatch;
    884             public boolean tagAll;
    885             public boolean valueAll;
    886             public boolean valueBool;
     934            Object tag;
     935            Object value;
     936            boolean noMatch;
     937            boolean tagAll;
     938            boolean valueAll;
     939            boolean valueBool;
    887940
    888941            private static Pattern getPattern(String str) {
    … …  
    895948            }
    896949
    897             public CheckerElement(String exp) {
     950            CheckerElement(String exp) {
    898951                Matcher m = Pattern.compile("(.+)([!=]=)(.+)").matcher(exp);
    899952                m.matches();
    … …  
    907960                    noMatch = "!=".equals(m.group(2));
    908961                    n = m.group(3).trim();
    909                     if ("*".equals(n)) {
    910                         valueAll = true;
    911                     } else if ("BOOLEAN_TRUE".equals(n)) {
     962                    switch (n) {
     963                    case "*": valueAll = true; break;
     964                    case "BOOLEAN_TRUE":
    912965                        valueBool = true;
    913966                        value = OsmUtils.TRUE_VALUE;
    914                     } else if ("BOOLEAN_FALSE".equals(n)) {
     967                        break;
     968                    case "BOOLEAN_FALSE":
    915969                        valueBool = true;
    916970                        value = OsmUtils.FALSE_VALUE;
    917                     } else {
     971                        break;
     972                    default:
    918973                        value = n.startsWith("/") ? getPattern(n) : n;
    919974                    }
    … …  
    921976            }
    922977
    923             public boolean match(Map<String, String> keys) {
     978            boolean match(Map<String, String> keys) {
    924979                for (Entry<String, String> prop: keys.entrySet()) {
    925980                    String key = prop.getKey();
Note: See TracChangeset for help on using the changeset viewer.