Changeset 15103 in josm


Ignore:
Timestamp:
2019-05-21T17:02:31+02:00 (5 months ago)
Author:
GerdP
Message:

see #14287: improve results when Test.partialSelection is true. This happens when either elements are selected or when the test is performed before uploading data.
In a final step the surrounding objects of the tested elements are also tested with a reduced set of rules which will find problems like "amenity inside amenity".

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

    r15102 r15103  
    7070import org.openstreetmap.josm.gui.mappaint.mapcss.Selector;
    7171import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.AbstractSelector;
    72 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.ChildOrParentSelector;
    73 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.ChildOrParentSelectorType;
    7472import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.GeneralSelector;
    7573import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.OptimizedGeneralSelector;
     
    10098public class MapCSSTagChecker extends Test.TagTest {
    10199    IndexData indexData;
     100    final Set<OsmPrimitive> tested = new HashSet<>();
     101
     102    private static final boolean ALL_TESTS = true;
     103    private static final boolean ONLY_SELECTED_TESTS = false;
    102104
    103105    /**
     
    130132        final MapCSSRuleIndex multipolygonRules = new MapCSSRuleIndex();
    131133
    132         private IndexData(MultiMap<String, TagCheck> checks, boolean includeOtherSeverity) {
    133             buildIndex(checks, includeOtherSeverity);
    134         }
    135 
    136         private void buildIndex(MultiMap<String, TagCheck> checks, boolean includeOtherSeverity) {
     134        private IndexData(MultiMap<String, TagCheck> checks, boolean includeOtherSeverity, boolean allTests) {
     135            buildIndex(checks, includeOtherSeverity, allTests);
     136        }
     137
     138        private void buildIndex(MultiMap<String, TagCheck> checks, boolean includeOtherSeverity, boolean allTests) {
    137139            List<TagCheck> allChecks = new ArrayList<>();
    138140            for (Set<TagCheck> cs : checks.values()) {
     
    157159                for (Selector s : c.rule.selectors) {
    158160                    // find the rightmost selector, this must be a GeneralSelector
     161                    boolean hasLeftRightSel = false;
    159162                    Selector selRightmost = s;
    160163                    while (selRightmost instanceof Selector.ChildOrParentSelector) {
     164                        hasLeftRightSel = true;
    161165                        selRightmost = ((Selector.ChildOrParentSelector) selRightmost).right;
    162166                    }
     167                    if (!allTests && !hasLeftRightSel) {
     168                        continue;
     169                    }
     170
    163171                    MapCSSRule optRule = new MapCSSRule(s.optimizedBaseCheck(), c.rule.declaration);
    164172
     
    853861        final List<TestError> res = new ArrayList<>();
    854862        if (indexData == null)
    855             indexData = new IndexData(checks, includeOtherSeverity);
     863            indexData = new IndexData(checks, includeOtherSeverity, ALL_TESTS);
    856864
    857865        MapCSSRuleIndex matchingRuleIndex = indexData.get(p);
     
    865873            MapCSSRule r = candidates.next();
    866874            env.clearSelectorMatchingInformation();
    867             if (partialSelection && r.selector instanceof Selector.ChildOrParentSelector) {
    868                 ChildOrParentSelector sel = (Selector.ChildOrParentSelector) r.selector;
    869                 boolean needEnclosing = sel.type == ChildOrParentSelectorType.SUBSET_OR_EQUAL
    870                         || sel.type == ChildOrParentSelectorType.NOT_SUBSET_OR_EQUAL;
    871                 if (needEnclosing && p.getDataSet() != null) {
    872                     List<OsmPrimitive> toCheck = new ArrayList<>();
    873                     toCheck.addAll(p.getDataSet().searchWays(p.getBBox()));
    874                     toCheck.addAll(p.getDataSet().searchRelations(p.getBBox()));
    875                     toCheck.removeIf(OsmPrimitive::isSelected);
    876                     if (!toCheck.isEmpty()) {
    877                         Set<Set<TagCheck>> checksCol = Collections.singleton(Collections.singleton(indexData.getCheck(r)));
    878                         for (OsmPrimitive p2 : toCheck) {
    879                             for (TestError e : getErrorsForPrimitive(p2, includeOtherSeverity, checksCol)) {
    880                                 if (e.getPrimitives().contains(p)) {
    881                                     addIfNotSimilar(e, res);
    882                                 }
    883                             }
    884                         }
    885                     }
    886                 }
    887             }
    888875            if (r.selector.matches(env)) { // as side effect env.parent will be set (if s is a child selector)
    889876                TagCheck check = indexData.getCheck(r);
     
    963950        for (TestError e : getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get())) {
    964951            addIfNotSimilar(e, errors);
     952        }
     953        if (partialSelection && p.isTagged()) {
     954            tested.add(p);
    965955        }
    966956    }
     
    11461136
    11471137    @Override
    1148     public void startTest(ProgressMonitor progressMonitor) {
     1138    public synchronized void startTest(ProgressMonitor progressMonitor) {
    11491139        super.startTest(progressMonitor);
    11501140        super.setShowElements(true);
    11511141        if (indexData == null) {
    1152             indexData = new IndexData(checks, ValidatorPrefHelper.PREF_OTHER.get());
    1153         }
     1142            indexData = new IndexData(checks, includeOtherSeverityChecks(), ALL_TESTS);
     1143        }
     1144        tested.clear();
    11541145    }
    11551146
    11561147    @Override
    1157     public void endTest() {
     1148    public synchronized void endTest() {
     1149        if (partialSelection && !tested.isEmpty()) {
     1150            // #14287: see https://josm.openstreetmap.de/ticket/14287#comment:15
     1151            // execute tests for objects which might contain or cross previously tested elements
     1152
     1153            // rebuild index with a reduced set of rules (those that use ChildOrParentSelector) and thus may have left selectors
     1154            // matching the previously tested elements
     1155            indexData = new IndexData(checks, includeOtherSeverityChecks(), ONLY_SELECTED_TESTS);
     1156
     1157            Set<OsmPrimitive> surrounding = new HashSet<>();
     1158            for (OsmPrimitive p : tested) {
     1159                if (p.getDataSet() != null) {
     1160                    surrounding.addAll(p.getDataSet().searchWays(p.getBBox()));
     1161                    surrounding.addAll(p.getDataSet().searchRelations(p.getBBox()));
     1162                }
     1163            }
     1164            final boolean includeOtherSeverity = includeOtherSeverityChecks();
     1165            for (OsmPrimitive p : surrounding) {
     1166                if (tested.contains(p) )
     1167                    continue;
     1168                Collection<TestError> additionalErrors = getErrorsForPrimitive(p, includeOtherSeverity);
     1169                for (TestError e : additionalErrors) {
     1170                    if (e.getPrimitives().stream().anyMatch(tested::contains))
     1171                        addIfNotSimilar(e, errors);
     1172                }
     1173            }
     1174            tested.clear();
     1175        }
    11581176        super.endTest();
    11591177        // no need to keep the index, it is quickly build and doubles the memory needs
     
    11611179    }
    11621180
     1181    private boolean includeOtherSeverityChecks() {
     1182        return isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get();
     1183    }
     1184
    11631185}
Note: See TracChangeset for help on using the changeset viewer.