Not capturing all elements when using ∈

[naoliv]

Validate the attached example file and see only one warning about amenity inside amenity - amenity=parking inside amenity=parking

Since we have 6 identical nodes, we should see 6 objects with a warning (and not only one).

JOSM:

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-01-25 02:17:36 +0100 (Wed, 25 Jan 2017)
Build-Date:2017-01-25 02:33:55
Revision:11490
Relative:URL: ^/trunk

Identification: JOSM/1.5 (11490 en) Linux Debian GNU/Linux 9.0 (stretch)
Memory Usage: 271 MB / 10206 MB (111 MB allocated, but free)
Java version: 1.8.0_111-8u111-b14-3-b14, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1600x900, :0.1 1280x1024
Maximum Screen Size: 1600x1024
Java package: openjdk-8-jre:amd64-8u111-b14-3
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [-Dawt.useSystemAAFontSettings=on]
Program arguments: [--language=en]
Dataset consistency test: No problems found

[naoliv]

The test here is JOSM's node[amenity=parking] ∈ *[amenity=parking] { (...) but doing some local tests, the problem seems to be related with ∈

[Don-vip]

In 11491/josm:

see #14287 - add non regression unit test

[Don-vip]

@Simon: This is a side effect of performance improvements done in r6609, especially this part in Selector.AbstractFinder.visit:

            public void visit(Collection<? extends OsmPrimitive> primitives) {
                for (OsmPrimitive p : primitives) {
                    if (e.child != null) {
                        // abort if first match has been found
                        break;
                    } else if (isPrimitiveUsable(p)) {
                        p.accept(this);
                    }
                }
            }

I have added a unit test to quickly debug the problem. Any hint about how to resolve this issue without a severe performance drawback?

[Don-vip]

In 11492/josm:

see #14287 - add non regression unit test (forgot test file)

[GerdP]

I wonder if we should produce 6 warnings or one warning with 7 objects?

[GerdP]

It would be a bit more work to produce multiple warnings for each. With the attached patch you get

[GerdP]

In 15064/josm:

fix #12627,#14287,#14289,#17695

• let CrossingFinder and ContainsFinder find all objects instead of stopping at first match
• if objects are selected, make sure that ContainsFinder is called for enclosing objects which are not in the selection
• enable corresponding unit tests

Draw back: MapCSSTagChecker is a bit slower.

[GerdP]

Replying to GerdP:

• if objects are selected, make sure that ContainsFinder is called for enclosing objects which are not in the selection

Unfortunately my change only works in some special cases where also the right selector is a possible match for the selected element. The "amenity=parking inside amenity=parking" is one of them as the right and left selectors both match for a node with amenity=parking.
The following will not work:

*[building] ∈ area[natural=water] {
  throwWarning: tr("Building inside Water Area");
}

when the building is selected. I try to find a solution for that as well.

[GerdP]

The patch improves results when Test.partialSelection is true. This happens when either elements are selected or when the test is performed before uploading data.
Problem: In some cases it requires seconds to performs the additional tests, for example if someone selects many objects and executes the validator.
I am working on a better solution which rebuilds the index so that only those tests which are needed to find "enclosing" objects are executed...

[GerdP]

In 15103/josm:

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".