Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/OSMValidatorPlugin.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/OSMValidatorPlugin.java (revision 3037) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/OSMValidatorPlugin.java (revision 3038) @@ -3,4 +3,5 @@ import static org.openstreetmap.josm.tools.I18n.tr; +import java.lang.reflect.InvocationTargetException; import java.util.*; import java.util.regex.Matcher; @@ -11,4 +12,5 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.UploadAction; +import org.openstreetmap.josm.actions.UploadAction.UploadHook; import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.gui.preferences.PreferenceSetting; @@ -43,9 +45,12 @@ DuplicateSegment.class, SingleNodeSegment.class, + UntaggedNode.class, TaggedSegment.class, + UntaggedWay.class, UnorderedWay.class, SpellCheck.class, - UntaggedNode.class, OrphanSegment.class, + ReusedSegment.class, + CrossingSegments.class, }; @@ -73,13 +78,27 @@ newFrame.addToggleDialog(validationDialog); Main.main.addLayer(new ErrorLayer(tr("Validation errors"))); - try + } + + // Add/Remove the upload hook + try + { + LinkedList hooks = ((UploadAction)Main.main.menu.upload).uploadHooks; + Iterator hooksIt = hooks.iterator(); + while( hooksIt.hasNext() ) { - ((UploadAction)Main.main.menu.upload).uploadHooks.add( 0, new ValidateUploadHook() ); + if( hooksIt.next() instanceof ValidateUploadHook ) + { + if( newFrame == null ) + hooksIt.remove(); + break; + } } - catch(Throwable t) - { - // JOSM has no upload hooks in older versions - } - } + if( newFrame != null ) + hooks.add( 0, new ValidateUploadHook() ); + } + catch(Throwable t) + { + // JOSM has no upload hooks in older versions + } } @@ -118,5 +137,5 @@ String simpleName = testClass.getSimpleName(); - test.testBeforeUpload = Main.pref.getBoolean( "tests." + simpleName + ".checkBeforeUpload"); + test.testBeforeUpload = Main.pref.getBoolean( "tests." + simpleName + ".checkBeforeUpload", true); enabledTests.put(simpleName, test); } @@ -165,9 +184,14 @@ } } - catch(Exception e) - { - e.printStackTrace(); - JOptionPane.showMessageDialog(null, tr("Error initializing test {0}.", test.getClass().getSimpleName())); - } + catch(InvocationTargetException ite) + { + ite.getCause().printStackTrace(); + JOptionPane.showMessageDialog(null, tr("Error initializing test {0}:\n {1}", test.getClass().getSimpleName(), ite.getCause().getMessage())); + } + catch(Exception e) + { + e.printStackTrace(); + JOptionPane.showMessageDialog(null, tr("Error initializing test {0}:\n {1}", test.getClass().getSimpleName(), e)); + } } } Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/CrossingSegments.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/CrossingSegments.java (revision 3038) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/CrossingSegments.java (revision 3038) @@ -0,0 +1,167 @@ +package org.openstreetmap.josm.plugins.validator.tests; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.awt.Point; +import java.awt.geom.Line2D; +import java.awt.geom.Point2D; +import java.util.*; + +import org.openstreetmap.josm.data.coor.EastNorth; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.*; +import org.openstreetmap.josm.plugins.validator.Severity; +import org.openstreetmap.josm.plugins.validator.Test; +import org.openstreetmap.josm.plugins.validator.TestError; + +/** + * Tests if there are segments that crosses in the same layer + * + * @author frsantos + */ +public class CrossingSegments extends Test +{ + /** All segments, grouped by cells */ + Map> cellSegments; + + /** + * Constructor + */ + public CrossingSegments() + { + super(tr("Crossing roads."), + tr("This test checks if two roads crosses in the same layer, but are not connected by a node.")); + } + + + @Override + public void startTest() + { + cellSegments = new HashMap>(1000); + } + + @Override + public void endTest() + { + cellSegments = null; + } + + @Override + public void visit(Way w) + { + + if( w.deleted || w.get("highway") == null) + return; + + String layer1 = w.get("layer"); + for(Segment s : w.segments) + { + if( s.incomplete ) + continue; + + ExtendedSegment es1 = new ExtendedSegment(s, layer1); + List> cellSegments = getSegments(s); + for( List segments : cellSegments) + { + for( ExtendedSegment es2 : segments) + { + String layer2 = es2.layer; + if( (layer1 == null && layer2 == null || layer1 != null && layer1.equals(layer2)) && + es1.intersects(es2)) + { + List primitives = new ArrayList(); + primitives.add(es1.s); + primitives.add(es2.s); + errors.add( new TestError(this, Severity.WARNING, tr("Crossing roads"), primitives) ); + } + } + segments.add(es1); + } + } + } + + /** + * Returns all the cells this segment crosses . Each cell contains the list + * of segments already processed + * + * @param s + * The segment + * @return A list with all the cells the segment crosses. + */ + public List> getSegments(Segment s) + { + List> cells = new ArrayList>(); + long x0 = Math.round(s.from.eastNorth.east()*100); + long x1 = Math.round(s.to.eastNorth.east()*100); + for( ; x0<=x1; x0++) + { + long y0 = Math.round(s.from.eastNorth.north()*100); + long y1 = Math.round(s.to.eastNorth.north()*100); + for( ; y0<=y1; y0++) + { + Point2D p = new Point2D.Double(x0, y0); + List segments = cellSegments.get( p ); + if( segments == null ) + { + segments = new ArrayList(); + cellSegments.put(p, segments); + } + + cells.add(segments); + } + } + + return cells; + } + + /** + * A segment is a line with the formula "y = a*x+b" + * @author frsantos + */ + class ExtendedSegment + { + /** The segment */ + Segment s; + + /** The layer */ + String layer; + + /** + * Constructor + * @param s The segment + * @param layer The layer of the way this segment is in + */ + public ExtendedSegment(Segment s, String layer) + { + this.s = s; + this.layer = layer; + } + + /** + * Checks whether this segment crosses other segment + * @param s2 The other segment + * @return true if both segements crosses + */ + public boolean intersects(ExtendedSegment s2) + { + Node n1From = this.s.from; + Node n1To = this.s.to; + Node n2From = s2.s.from; + Node n2To = s2.s.to; + if( n1From.equals(n2From) || n1To.equals(n2To) || + n1From.equals(n2To) || n1To.equals(n2From) ) + { + return false; + } + + return Line2D.linesIntersect(n1From.eastNorth.east(), + n1From.eastNorth.north(), + n1To.eastNorth.east(), + n1To.eastNorth.north(), + n2From.eastNorth.east(), + n2From.eastNorth.north(), + n2To.eastNorth.east(), + n2To.eastNorth.north()); + } + } +} Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/ReusedSegment.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/ReusedSegment.java (revision 3038) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/ReusedSegment.java (revision 3038) @@ -0,0 +1,57 @@ +package org.openstreetmap.josm.plugins.validator.tests; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.List; +import java.util.Map; + +import org.openstreetmap.josm.data.osm.*; +import org.openstreetmap.josm.plugins.validator.*; +import org.openstreetmap.josm.plugins.validator.util.Bag; +/** + * Tests if there are duplicate segments + * + * @author frsantos + */ +public class ReusedSegment extends Test +{ + /** Bag of all segments */ + Bag segments; + + /** + * Constructor + */ + public ReusedSegment() + { + super(tr("Reused segments."), + tr("This test checks if a segment is used in more than one way.")); + } + + + @Override + public void startTest() + { + segments = new Bag(1000); + } + + @Override + public void endTest() + { + for(Map.Entry> entry : segments.entrySet() ) + { + Segment s = entry.getKey(); + if( entry.getValue().size() > 1) + { + errors.add( new TestError(this, Severity.OTHER, tr("Reused segments"), s) ); + } + } + segments = null; + } + + @Override + public void visit(Way w) + { + for( Segment s : w.segments) + if( !s.deleted ) segments.add( s, w ); + } +} Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/SpellCheck.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/SpellCheck.java (revision 3037) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/SpellCheck.java (revision 3038) @@ -46,4 +46,6 @@ /** Preference name for checking values */ public static final String PREF_CHECK_KEYS = "tests." + SpellCheck.class.getSimpleName() + ".checkKeys"; + /** Preference name for checking FIXMES */ + public static final String PREF_CHECK_FIXMES = "tests." + SpellCheck.class.getSimpleName() + ".checkFixmes"; /** Preference name for sources */ public static final String PREF_SOURCES = "tests." + SpellCheck.class.getSimpleName() + ".sources"; @@ -54,4 +56,6 @@ /** Preference name for values upload check */ public static final String PREF_CHECK_VALUES_BEFORE_UPLOAD = "tests." + SpellCheck.class.getSimpleName() + ".checkValuesBeforeUpload"; + /** Preference name for fixmes upload check */ + public static final String PREF_CHECK_FIXMES_BEFORE_UPLOAD = "tests." + SpellCheck.class.getSimpleName() + ".checkFixmesBeforeUpload"; /** Whether to check keys */ @@ -59,4 +63,6 @@ /** Whether to check values */ protected boolean checkValues = false; + /** Whether to check for fixmes in values */ + protected boolean checkFixmes = false; /** Preferences checkbox for keys */ @@ -64,8 +70,12 @@ /** Preferences checkbox for values */ protected JCheckBox prefCheckValues; + /** Preferences checkbox for FIXMES */ + protected JCheckBox prefCheckFixmes; /** The preferences checkbox for validation of keys on upload */ protected JCheckBox prefCheckKeysBeforeUpload; /** The preferences checkbox for validation of values on upload */ protected JCheckBox prefCheckValuesBeforeUpload; + /** The preferences checkbox for validation of fixmes on upload */ + protected JCheckBox prefCheckFixmesBeforeUpload; /** The add button */ protected JButton addSrcButton; @@ -79,6 +89,8 @@ /** Invalid key error */ protected static int INVALID_KEY = 1; - /** Invalid value error */ - protected static int INVALID_VALUE = 2; + /** Invalid value error */ + protected static int INVALID_VALUE = 2; + /** fixme error */ + protected static int FIXME = 3; /** List of sources for spellcheck data */ @@ -89,4 +101,6 @@ /** Whether this test must check the values before upload. Used by peferences */ protected boolean testValuesBeforeUpload; + /** Whether this test must check form fixmes in values before upload. Used by peferences */ + protected boolean testFixmesBeforeUpload; /** @@ -95,6 +109,6 @@ public SpellCheck() { - super(tr("Properties spell checker."), - tr("This plugin checks misspelled property keys and values.")); + super(tr("Properties checker."), + tr("This plugin checks for errors in property keys and values.")); } @@ -124,8 +138,15 @@ StringTokenizer st = new StringTokenizer(sources, ";"); + StringBuilder errorSources = new StringBuilder(); while (st.hasMoreTokens()) { String source = st.nextToken(); File sourceFile = Util.mirror(new URL(source), plugin.getPluginDir()); + if( sourceFile == null || !sourceFile.exists() ) + { + errorSources.append(source).append("\n"); + continue; + } + BufferedReader reader = new BufferedReader(new FileReader(sourceFile)); @@ -154,4 +175,8 @@ while( true ); } + + if( errorSources.length() > 0 ) + throw new IOException( tr("Could not download spellcheck data file:\n {0}", errorSources) ); + } @@ -223,13 +248,21 @@ withErrors.add(p, "IPK"); } - if( checkValues && value != null && value.length() > 0 ) - { - List values = spellCheckValueData.get(key); - if( values != null && !values.contains(prop.getValue()) && !withErrors.contains(p, "UPV")) - { - errors.add( new TestError(this, Severity.OTHER, tr("Unknown property values"), p, INVALID_VALUE) ); - withErrors.add(p, "UPV"); - } - } + if( checkValues && value != null && value.length() > 0 && spellCheckValueData != null) + { + List values = spellCheckValueData.get(key); + if( values != null && !values.contains(prop.getValue()) && !withErrors.contains(p, "UPV")) + { + errors.add( new TestError(this, Severity.OTHER, tr("Unknown property values"), p, INVALID_VALUE) ); + withErrors.add(p, "UPV"); + } + } + if( checkFixmes && value != null && value.length() > 0 ) + { + if( value.contains("FIXME") && !withErrors.contains(p, "FIXME")) + { + errors.add( new TestError(this, Severity.OTHER, tr("FIXMES"), p, FIXME) ); + withErrors.add(p, "FIXME"); + } + } } } @@ -312,9 +345,13 @@ checkKeys = Main.pref.getBoolean(PREF_CHECK_KEYS); if( isBeforeUpload ) - checkKeys = checkKeys && Main.pref.getBoolean(PREF_CHECK_KEYS_BEFORE_UPLOAD); + checkKeys = checkKeys && Main.pref.getBoolean(PREF_CHECK_KEYS_BEFORE_UPLOAD, true); checkValues = Main.pref.getBoolean(PREF_CHECK_VALUES); if( isBeforeUpload ) - checkValues = checkValues && Main.pref.getBoolean(PREF_CHECK_VALUES_BEFORE_UPLOAD); + checkValues = checkValues && Main.pref.getBoolean(PREF_CHECK_VALUES_BEFORE_UPLOAD, true); + + checkFixmes = Main.pref.getBoolean(PREF_CHECK_FIXMES); + if( isBeforeUpload ) + checkFixmes = checkFixmes && Main.pref.getBoolean(PREF_CHECK_FIXMES_BEFORE_UPLOAD, true); } @@ -331,5 +368,5 @@ testPanel.add( new JLabel(), GBC.eol()); - boolean checkKeys = Main.pref.getBoolean(PREF_CHECK_KEYS); + boolean checkKeys = Main.pref.getBoolean(PREF_CHECK_KEYS, true); prefCheckKeys = new JCheckBox(tr("Check property keys."), checkKeys); prefCheckKeys .setToolTipText(tr("Validate that property keys are valid checking against list of words.")); @@ -337,5 +374,5 @@ prefCheckKeysBeforeUpload = new JCheckBox(); - prefCheckKeysBeforeUpload.setSelected(Main.pref.getBoolean(PREF_CHECK_KEYS_BEFORE_UPLOAD)); + prefCheckKeysBeforeUpload.setSelected(Main.pref.getBoolean(PREF_CHECK_KEYS_BEFORE_UPLOAD, true)); testPanel.add(prefCheckKeysBeforeUpload, GBC.eop().insets(20,0,0,0)); @@ -411,5 +448,5 @@ buttonPanel.setEnabled( checkKeys ); - boolean checkValues = Main.pref.getBoolean(PREF_CHECK_VALUES); + boolean checkValues = Main.pref.getBoolean(PREF_CHECK_VALUES, true); prefCheckValues = new JCheckBox(tr("Check property values."), checkValues); prefCheckValues .setToolTipText(tr("Validate that property values are valid checking against presets.")); @@ -417,7 +454,15 @@ prefCheckValuesBeforeUpload = new JCheckBox(); - prefCheckValuesBeforeUpload.setSelected(Main.pref.getBoolean(PREF_CHECK_VALUES_BEFORE_UPLOAD)); + prefCheckValuesBeforeUpload.setSelected(Main.pref.getBoolean(PREF_CHECK_VALUES_BEFORE_UPLOAD, true)); testPanel.add(prefCheckValuesBeforeUpload, GBC.eop().insets(20,0,0,0)); - + + boolean checkFixmes = Main.pref.getBoolean(PREF_CHECK_FIXMES, true); + prefCheckFixmes = new JCheckBox(tr("Check for FIXMES."), checkFixmes); + prefCheckFixmes.setToolTipText(tr("Looks for nodes, segments or ways with FIXME in any property value.")); + testPanel.add(prefCheckFixmes, GBC.std().insets(40,0,0,0)); + + prefCheckFixmesBeforeUpload = new JCheckBox(); + prefCheckFixmesBeforeUpload.setSelected(Main.pref.getBoolean(PREF_CHECK_FIXMES_BEFORE_UPLOAD, true)); + testPanel.add(prefCheckFixmesBeforeUpload, GBC.eop().insets(20,0,0,0)); } @@ -432,4 +477,6 @@ prefCheckValues.setEnabled(enabled); prefCheckValuesBeforeUpload.setEnabled(enabled); + prefCheckFixmes.setEnabled(enabled); + prefCheckFixmesBeforeUpload.setEnabled(enabled); } @@ -439,7 +486,9 @@ Main.pref.put(PREF_CHECK_VALUES, prefCheckValues.isSelected()); Main.pref.put(PREF_CHECK_KEYS, prefCheckKeys.isSelected()); + Main.pref.put(PREF_CHECK_FIXMES, prefCheckFixmes.isSelected()); Main.pref.put(PREF_CHECK_VALUES_BEFORE_UPLOAD, prefCheckValuesBeforeUpload.isSelected()); Main.pref.put(PREF_CHECK_KEYS_BEFORE_UPLOAD, prefCheckKeysBeforeUpload.isSelected()); - Main.pref.put(PREF_CHECK_BEFORE_UPLOAD, prefCheckKeysBeforeUpload.isSelected() || prefCheckValuesBeforeUpload.isSelected()); + Main.pref.put(PREF_CHECK_FIXMES_BEFORE_UPLOAD, prefCheckFixmesBeforeUpload.isSelected()); + Main.pref.put(PREF_CHECK_BEFORE_UPLOAD, prefCheckKeysBeforeUpload.isSelected() || prefCheckValuesBeforeUpload.isSelected() || prefCheckFixmesBeforeUpload.isSelected()); String sources = ""; if( spellcheckSources.getModel().getSize() > 0 ) Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/UntaggedWay.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/UntaggedWay.java (revision 3038) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/tests/UntaggedWay.java (revision 3038) @@ -0,0 +1,62 @@ +package org.openstreetmap.josm.plugins.validator.tests; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.Map; + +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.plugins.validator.*; +/** + * Checks for untagged ways + * + * @author frsantos + */ +public class UntaggedWay extends Test +{ + /** Tags allowed in a way */ + public static String[] allowedTags = new String[] { "created_by", "converted_by" }; + + /** + * Constructor + */ + public UntaggedWay() + { + super(tr("Untagged ways."), + tr("This test checks for untagged ways.")); + } + + @Override + public void visit(Way w) + { + int numTags = 0; + Map tags = w.keys; + if( tags != null ) + { + numTags = tags.size(); + for( String tag : allowedTags) + if( tags.containsKey(tag) ) numTags--; + + if( numTags != 0 && tags.containsKey("highway" ) ) + { + if( !tags.containsKey("name") && !tags.containsKey("ref") ) + { + boolean hasName = false; + for( String key : w.keySet()) + { + hasName = key.startsWith("name:") || key.endsWith("_name") || key.endsWith("_ref"); + if( hasName ) + break; + } + + if( !hasName) + errors.add( new TestError(this, Severity.WARNING, tr("Unnamed ways"), w) ); + } + } + } + + if( numTags == 0 ) + { + errors.add( new TestError(this, Severity.WARNING, tr("Untagged ways"), w) ); + } + } +} Index: /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/util/Util.java =================================================================== --- /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/util/Util.java (revision 3037) +++ /applications/editors/josm/plugins/validator/src/org/openstreetmap/josm/plugins/validator/util/Util.java (revision 3038) @@ -6,4 +6,5 @@ import java.io.*; import java.net.URL; +import java.net.URLConnection; import java.util.StringTokenizer; import java.util.regex.Matcher; @@ -172,7 +173,6 @@ * @param destDir The destionation dir of the mirrored file * @return The local file - * @throws IOException If any error reading or writing the file - */ - public static File mirror(URL url, String destDir) throws IOException + */ + public static File mirror(URL url, String destDir) { if( url.getProtocol().equals("file") ) @@ -199,5 +199,7 @@ try { - bis = new BufferedInputStream(url.openStream()); + URLConnection conn = url.openConnection(); + conn.setConnectTimeout(5000); + bis = new BufferedInputStream(conn.getInputStream()); bos = new BufferedOutputStream( new FileOutputStream(localPath) ); byte[] buffer = new byte[4096]; @@ -207,4 +209,11 @@ bos.write( buffer, 0, length ); } + } + catch(IOException ioe) + { + if( oldFile != null ) + return oldFile; + else + return null; } finally