Modify

Opened 22 months ago

Last modified 3 hours ago

#19648 new enhancement

[patch] Show warning on invalid MapCSS validator rule parsing

Reported by: skyper Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: template_report rule validation mapcss class Cc:

Description

What steps will reproduce the problem?

  1. Have a typo in a validator mapcss rule file, like the missing y below:
    way[highway =~ /^(motorway|motorway_link|trunk|trunk_link)$/] {
      set MotorwayTrunk;
    }
    way.MotorwaTrunk[lanes][turn:lanes              ][tag(lanes)!=eval(count(split("|", tag("turn:lanes"))))],
    way.MotorwaTrunk[lanes][change:lanes            ][tag(lanes)!=eval(count(split("|", tag("change:lanes"))))],
    way.MotorwaTrunk[lanes][maxspeed:lanes          ][tag(lanes)!=eval(count(split("|", tag("maxspeed:lanes"))))],
    way.MotorwaTrunk[lanes][minspeed:lanes          ][tag(lanes)!=eval(count(split("|", tag("minspeed:lanes"))))],
    way.MotorwaTrunk[lanes][destination:lanes       ][tag(lanes)!=eval(count(split("|", tag("destination:lanes"))))],
    way.MotorwaTrunk[lanes][destination:ref:lanes   ][tag(lanes)!=eval(count(split("|", tag("destination:ref:lanes"))))],
    way.MotorwaTrunk[lanes][destination:symbol:lanes][tag(lanes)!=eval(count(split("|", tag("destination:symbol:lanes"))))] {
        throwWarning: tr("Different number of lanes in the keys {0} and {1}", "{1.key}", "{2.key}");
        group: tr("suspicious tag combination");
    }
    
  2. (Re-)Load file

What is the expected result?

A warning about an unknown class in use.

What happens instead?

No warning

Please provide any additional information below. Attach a screenshot if possible.

Some Information about existing but not use classes would be nice, too.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-08-06 21:47:58 +0200 (Thu, 06 Aug 2020)
Revision:16851
Build-Date:2020-08-07 01:30:47
URL:https://josm.openstreetmap.de/svn/trunk

Attachments (1)

josm_19648.patch (2.3 KB) - added by gaben 4 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 months ago by gaben

Summary: MapCSS Rule validation: Warn about unknown class.Show warning on invalid MapCSS validator rule parsing

comment:2 Changed 4 months ago by gaben

While experimenting with a custom MapCSS rule, I noticed parsing of invalid rules silently fail on the GUI side. In case of error, the console prints detailed information. Add this or parts to a warning dialog to help diagnose the problem.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2022-01-09 17:28:53 +0100 (Sun, 09 Jan 2022)
Build-Date:2022-01-10 02:31:03
Revision:18364
Relative:URL: ^/trunk

comment:3 Changed 4 months ago by gaben

The issue is coming from the MapCSSParser.jj file. I have a patch that makes JOSM print the messages but at the same time introduces some GUI exceptions (EDT violations).

comment:4 Changed 4 months ago by gaben

Summary: Show warning on invalid MapCSS validator rule parsing[WIP patch] Show warning on invalid MapCSS validator rule parsing
  • src/org/openstreetmap/josm/gui/mappaint/mapcss/MapCSSParser.jj

     
    1414import java.util.Collections;
    1515import java.util.List;
    1616
     17import javax.swing.JOptionPane;
     18
    1719import org.openstreetmap.josm.data.preferences.NamedColorProperty;
     20import org.openstreetmap.josm.gui.MainApplication;
    1821import org.openstreetmap.josm.gui.mappaint.Keyword;
    1922import org.openstreetmap.josm.gui.mappaint.Range;
    2023import org.openstreetmap.josm.gui.mappaint.mapcss.Condition;
     
    7881            this.idx = idx;
    7982        }
    8083    }
    81    
     84
    8285    /**
    8386     * Constructor which initializes the parser with a certain lexical state.
    8487     * @param in input
     
    11641167   
    11651168    Logging.error("Skipping to the next rule, because of an error:");
    11661169    Logging.error(e);
     1170
     1171    JOptionPane.showMessageDialog(MainApplication.getMainFrame(), e.getMessage(), "MapCSS rule parsing error", JOptionPane.ERROR_MESSAGE);
    11671172    if (sheet != null) {
    11681173        sheet.logError(e);
    11691174    }

The patch introduces some EDT violations, which needs to be solved first. If someone knows how to fix it, please do it.

comment:5 Changed 4 months ago by GerdP

See GuiHelper.runInEDT() or one of the variants.

Changed 4 months ago by gaben

Attachment: josm_19648.patch added

comment:6 Changed 4 months ago by gaben

Summary: [WIP patch] Show warning on invalid MapCSS validator rule parsing[patch] Show warning on invalid MapCSS validator rule parsing

Thank you @GerdP! Now the EDT violations are gone. I think the patch is good enough, but please review it.

comment:7 Changed 4 months ago by GerdP

You can simplify that to GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(..));
like in MainLayerManager:

            GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(MainApplication.getMainFrame(),
                    tr("Trying to delete the layer with background upload. Please wait until the upload is finished."))); 
}

comment:8 Changed 4 months ago by gaben

I tried that, but JavaCC didn't like it :/

comment:9 Changed 3 hours ago by taylor.smock

Looking at the release-notes.md, v7.0.5 "[added] support for Java7 language features". So I don't think Java 8 language features are supported.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to skyper
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


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

 
Note: See TracTickets for help on using tickets.