Modify

Opened 4 years ago

Closed 4 years ago

#2868 closed defect (fixed)

tagchecker.cfg fails to throw errors when highway=secondary

Reported by: delta_foxtrot2 Owned by: team
Priority: major Component: Core validator
Version: Keywords:
Cc: delta_foxtrot@…

Description

This may not just be restricted to highway=secondary, but when testing regex in tagchecker.cfg I noticed that if I named a street Str. no warning is thrown, yet if I name a highway=residential Str. I get the warning I was expecting.

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by delta_foxtrot2

This check also fails for teritary and motorway, other highway types seem to be checked as expected.

comment:2 Changed 4 years ago by delta_foxtrot2

tertiary not teritary

comment:3 Changed 4 years ago by delta_foxtrot2

After having a quick look through code + the config file, I noticed this in tagcheck.cfg:

way  : I : highway == secondary && ref != *                    # highway without a reference
way  : I : highway == tertiary && ref != *                     # highway without a reference
way  : I : highway == motorway && nat_ref != *                 # highway without a reference

They just happen to be the exact same three highway types that don't trigger a warning about street abbreviations. I also ignored this warning, so I can only conclude the code that does the ignoring doesn't assign unique values for each error string in the config file

comment:4 Changed 4 years ago by delta_foxtrot2

I think the problem is in the code in TagChecker.java:

                    if((tagAll || (tag instanceof Pattern ? ((Pattern)tag).matcher(key).matches() : key.equals(tag)))
                    && (valueAll || (value instanceof Pattern ? ((Pattern)value).matcher(val).matches() : val.equals(value))))
                        return !noMatch;

However I don't understand fully what this code is doing, or not doing, when it comes to the regex matching, especially pairs of checks, and I have no idea how to fix this.

comment:5 Changed 4 years ago by delta_foxtrot2

Also it's reasonably simple to assign a unique code for each unique String, although there is still a small chance of overlap. I did a quick proof of concept hack using CRC32.

Change:

                description = m.group(1);
                if(description != null && description.length() == 0)
                    description = null;

to:

                description = m.group(1);
                if(description != null && description.length() == 0)
                    description = null;
                else
                {
                    java.util.zip.CRC32 x = new java.util.zip.CRC32();
                    x.update(description.getBytes());
                    TAG_CODE = (int)x.getValue();
                }

and:

        public int getCode()
        {
            return code + type;
        }

to:

        public int getCode()
        {
            if(TAG_CODE == 0)
                return code + type;
            return TAG_CODE;
        }

comment:6 Changed 4 years ago by delta_foxtrot2

Also if I change the order of checks in tagchecker.cfg it only shows the error about the street abbreviation and doesn't show the error about highway with no ref.

So this seems to be just a case of the first warning being recorded, even if that warning is suppressed, and subsequent warnings not being shown at all.

comment:7 Changed 4 years ago by stoecker

  • Resolution set to fixed
  • Status changed from new to closed

Fix in [o16438].

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed .
as The resolution will be set. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.