Changeset 14640 in josm


Ignore:
Timestamp:
2019-01-04T23:44:59+01:00 (5 years ago)
Author:
simon04
Message:

fix #17169 - Missing MessageFormat arguments in OptionParser

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/tools/OptionParser.java

    r14441 r14640  
    4242    public OptionParser addShortAlias(String optionName, String shortName) {
    4343        if (!shortName.matches("\\w")) {
    44             throw new IllegalArgumentException("Short name " + shortName + " must be one character");
     44            throw new IllegalArgumentException("Short name '" + shortName + "' must be one character");
    4545        }
    4646        if (availableOptions.containsKey("-" + shortName)) {
    47             throw new IllegalArgumentException("Short name " + shortName + " is already used");
     47            throw new IllegalArgumentException("Short name '" + shortName + "' is already used");
    4848        }
    4949        AvailableOption longDefinition = availableOptions.get("--" + optionName);
    … …  
    7070    private void checkOptionName(String optionName) {
    7171        if (!optionName.matches("\\w([\\w-]*\\w)?")) {
    72             throw new IllegalArgumentException("Illegal option name: " + optionName);
     72            throw new IllegalArgumentException("Illegal option name: '" + optionName + "'");
    7373        }
    7474        if (availableOptions.containsKey("--" + optionName)) {
    75             throw new IllegalArgumentException("The option --" + optionName + " is already registered");
     75            throw new IllegalArgumentException("The option '--" + optionName + "' is already registered");
    7676        }
    7777    }
    … …  
    164164                    } else {
    165165                        if (toHandle.isEmpty() || "--".equals(toHandle.getFirst())) {
    166                             throw new OptionParseException(tr("{0}: option ''{1}'' requires an argument", program));
     166                            throw new OptionParseException(tr("{0}: option ''{1}'' requires an argument", program, optionName));
    167167                        }
    168168                        parameter = toHandle.removeFirst();
    … …  
    179179        availableOptions.values().stream().distinct().forEach(def -> {
    180180            long count = options.stream().filter(p -> def.equals(p.option)).count();
     181            String optionName = availableOptions.entrySet().stream()
     182                    .filter(entry -> def.equals(entry.getValue()))
     183                    .map(Entry::getKey)
     184                    .findFirst()
     185                    .orElse("?");
    181186            if (count < def.getRequiredCount().min) {
    182187                // min may be 0 or 1 at the moment
    183                 throw new OptionParseException(tr("{0}: option ''{1}'' is required"));
     188                throw new OptionParseException(tr("{0}: option ''{1}'' is required", program, optionName));
    184189            } else if (count > def.getRequiredCount().max) {
    185190                // max may be 1 or MAX_INT at the moment
    186                 throw new OptionParseException(tr("{0}: option ''{1}'' may not appear multiple times"));
     191                throw new OptionParseException(tr("{0}: option ''{1}'' may not appear multiple times", program, optionName));
    187192            }
    188193        });
    … …  
    222227                return alternatives.get(0);
    223228            } else if (alternatives.size() > 1) {
    224                 throw new OptionParseException(tr("{0}: option ''{1}'' is ambiguous", program));
     229                throw new OptionParseException(tr("{0}: option ''{1}'' is ambiguous", program, optionName));
    225230            }
    226231        }

  • trunk/test/unit/org/openstreetmap/josm/tools/OptionParserTest.java

    r14435 r14640  
    1212import java.util.concurrent.atomic.AtomicReference;
    1313
     14import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
     15import org.junit.Rule;
    1416import org.junit.Test;
     17import org.junit.rules.ExpectedException;
     18import org.openstreetmap.josm.testutils.JOSMTestRules;
    1519import org.openstreetmap.josm.tools.OptionParser.OptionCount;
    1620import org.openstreetmap.josm.tools.OptionParser.OptionParseException;
    … …  
    2226public class OptionParserTest {
    2327
     28    /**
     29     * Rule used for tests throwing exceptions.
     30     */
     31    @Rule
     32    public ExpectedException thrown = ExpectedException.none();
     33
     34    /**
     35     * Setup test.
     36     */
     37    @Rule
     38    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     39    public JOSMTestRules test = new JOSMTestRules().i18n();
     40
    2441    // A reason for moving to jupiter...
    25     @Test(expected = OptionParseException.class)
     42    @Test
    2643    public void testEmptyParserRejectsLongopt() {
     44        thrown.expect(OptionParseException.class);
     45        thrown.expectMessage("test: unrecognized option '--long'");
    2746        new OptionParser("test").parseOptions(Arrays.asList("--long"));
    2847    }
    2948
    30     @Test(expected = OptionParseException.class)
     49    @Test
    3150    public void testEmptyParserRejectsShortopt() {
     51        thrown.expect(OptionParseException.class);
     52        thrown.expectMessage("test: unrecognized option '-s'");
    3253        new OptionParser("test").parseOptions(Arrays.asList("-s"));
    3354    }
    3455
    35     @Test(expected = OptionParseException.class)
     56    @Test
    3657    public void testParserRejectsWrongShortopt() {
     58        thrown.expect(OptionParseException.class);
     59        thrown.expectMessage("test: unrecognized option '-s'");
    3760        new OptionParser("test").addFlagParameter("test", this::nop).addShortAlias("test", "t")
    3861                .parseOptions(Arrays.asList("-s"));
    3962    }
    4063
    41     @Test(expected = OptionParseException.class)
     64    @Test
    4265    public void testParserRejectsWrongLongopt() {
     66        thrown.expect(OptionParseException.class);
     67        thrown.expectMessage("test: unrecognized option '--wrong'");
    4368        new OptionParser("test").addFlagParameter("test", this::nop).parseOptions(Arrays.asList("--wrong"));
    4469    }
    … …  
    5479    }
    5580
    56     @Test(expected = OptionParseException.class)
     81    @Test
    5782    public void testParserOptionFailsIfMissing() {
     83        thrown.expect(OptionParseException.class);
     84        thrown.expectMessage("test: unrecognized option '--test2'");
    5885        AtomicReference<String> argFound = new AtomicReference<>();
    5986        OptionParser parser = new OptionParser("test")
    … …  
    6390    }
    6491
    65     @Test(expected = OptionParseException.class)
     92    @Test
    6693    public void testParserOptionFailsIfMissingArgument() {
     94        thrown.expect(OptionParseException.class);
     95        thrown.expectMessage("test: unrecognized option '--test2'");
    6796        AtomicReference<String> argFound = new AtomicReference<>();
    6897        OptionParser parser = new OptionParser("test")
    … …  
    72101    }
    73102
    74     @Test(expected = OptionParseException.class)
     103    @Test
    75104    public void testParserOptionFailsIfMissing2() {
     105        thrown.expect(OptionParseException.class);
     106        thrown.expectMessage("test: option '--test' is required");
    76107        AtomicReference<String> argFound = new AtomicReference<>();
    77108        OptionParser parser = new OptionParser("test")
    … …  
    81112    }
    82113
    83     @Test(expected = OptionParseException.class)
     114    @Test
    84115    public void testParserOptionFailsIfTwice() {
     116        thrown.expect(OptionParseException.class);
     117        thrown.expectMessage("test: option '--test' may not appear multiple times");
    85118        AtomicReference<String> argFound = new AtomicReference<>();
    86119        OptionParser parser = new OptionParser("test")
    … …  
    90123    }
    91124
    92     @Test(expected = OptionParseException.class)
     125    @Test
    93126    public void testParserOptionFailsIfTwiceForAlias() {
     127        thrown.expect(OptionParseException.class);
     128        thrown.expectMessage("test: option '-t' may not appear multiple times");
    94129        AtomicReference<String> argFound = new AtomicReference<>();
    95130        OptionParser parser = new OptionParser("test")
    … …  
    100135    }
    101136
    102     @Test(expected = OptionParseException.class)
     137    @Test
    103138    public void testOptionalOptionFailsIfTwice() {
     139        thrown.expect(OptionParseException.class);
     140        thrown.expectMessage("test: option '--test' may not appear multiple times");
    104141        OptionParser parser = new OptionParser("test")
    105142                .addFlagParameter("test", this::nop);
    … …  
    107144    }
    108145
    109     @Test(expected = OptionParseException.class)
     146    @Test
    110147    public void testOptionalOptionFailsIfTwiceForAlias() {
     148        thrown.expect(OptionParseException.class);
     149        thrown.expectMessage("test: option '-t' may not appear multiple times");
    111150        OptionParser parser = new OptionParser("test")
    112151                .addFlagParameter("test", this::nop)
    … …  
    115154    }
    116155
    117     @Test(expected = OptionParseException.class)
     156    @Test
    118157    public void testOptionalOptionFailsIfTwiceForAlias2() {
     158        thrown.expect(OptionParseException.class);
     159        thrown.expectMessage("test: option '-t' may not appear multiple times");
    119160        OptionParser parser = new OptionParser("test")
    120161                .addFlagParameter("test", this::nop)
    … …  
    145186    }
    146187
    147     @Test(expected = OptionParseException.class)
     188    @Test
    148189    public void testLongArgumentsMissingOption() {
     190        thrown.expect(OptionParseException.class);
     191        thrown.expectMessage("test: option '--test' requires an argument");
    149192        OptionParser parser = new OptionParser("test")
    150193                .addArgumentParameter("test", OptionCount.REQUIRED, this::nop);
    … …  
    153196    }
    154197
    155     @Test(expected = OptionParseException.class)
     198    @Test
    156199    public void testLongArgumentsMissingOption2() {
     200        thrown.expect(OptionParseException.class);
     201        thrown.expectMessage("test: option '--test' requires an argument");
    157202        OptionParser parser = new OptionParser("test")
    158203                .addArgumentParameter("test", OptionCount.REQUIRED, this::nop);
    … …  
    161206    }
    162207
    163     @Test(expected = OptionParseException.class)
     208    @Test
    164209    public void testShortArgumentsMissingOption() {
     210        thrown.expect(OptionParseException.class);
     211        thrown.expectMessage("test: option '-t' requires an argument");
    165212        OptionParser parser = new OptionParser("test")
    166213                .addArgumentParameter("test", OptionCount.REQUIRED, this::nop)
    … …  
    170217    }
    171218
    172     @Test(expected = OptionParseException.class)
     219    @Test
    173220    public void testShortArgumentsMissingOption2() {
     221        thrown.expect(OptionParseException.class);
     222        thrown.expectMessage("test: option '-t' requires an argument");
    174223        OptionParser parser = new OptionParser("test")
    175224                .addArgumentParameter("test", OptionCount.REQUIRED, this::nop)
    … …  
    179228    }
    180229
    181     @Test(expected = OptionParseException.class)
     230    @Test
    182231    public void testLongFlagHasOption() {
     232        thrown.expect(OptionParseException.class);
     233        thrown.expectMessage("test: option '--test' does not allow an argument");
    183234        OptionParser parser = new OptionParser("test")
    184235                .addFlagParameter("test", this::nop);
    … …  
    187238    }
    188239
    189     @Test(expected = OptionParseException.class)
     240    @Test
    190241    public void testShortFlagHasOption() {
     242        thrown.expect(OptionParseException.class);
     243        thrown.expectMessage("test: option '-t' does not allow an argument");
    191244        OptionParser parser = new OptionParser("test")
    192245                .addFlagParameter("test", this::nop)
    … …  
    251304    }
    252305
    253     @Test(expected = OptionParseException.class)
     306    @Test
    254307    public void testAmbiguousAlternatives() {
     308        thrown.expect(OptionParseException.class);
     309        thrown.expectMessage("test: option '--fl' is ambiguous");
    255310        AtomicReference<String> argFound = new AtomicReference<>();
    256311        AtomicBoolean usedFlag = new AtomicBoolean();
    … …  
    265320    }
    266321
    267     @Test(expected = OptionParseException.class)
     322    @Test
    268323    public void testMultipleShort() {
     324        thrown.expect(OptionParseException.class);
     325        thrown.expectMessage("test: unrecognized option '-ft'");
    269326        AtomicReference<String> argFound = new AtomicReference<>();
    270327        AtomicBoolean usedFlag = new AtomicBoolean();
    … …  
    300357    }
    301358
    302     @Test(expected = IllegalArgumentException.class)
     359    @Test
    303360    public void testIllegalOptionName() {
     361        thrown.expect(IllegalArgumentException.class);
     362        thrown.expectMessage("Illegal option name: ''");
    304363        new OptionParser("test").addFlagParameter("", this::nop);
    305364    }
    306365
    307     @Test(expected = IllegalArgumentException.class)
     366    @Test
    308367    public void testIllegalOptionName2() {
     368        thrown.expect(IllegalArgumentException.class);
     369        thrown.expectMessage("Illegal option name: '-'");
    309370        new OptionParser("test").addFlagParameter("-", this::nop);
    310371    }
    311372
    312     @Test(expected = IllegalArgumentException.class)
     373    @Test
    313374    public void testIllegalOptionName3() {
     375        thrown.expect(IllegalArgumentException.class);
     376        thrown.expectMessage("Illegal option name: '-test'");
    314377        new OptionParser("test").addFlagParameter("-test", this::nop);
    315378    }
    316379
    317     @Test(expected = IllegalArgumentException.class)
     380    @Test
    318381    public void testIllegalOptionName4() {
     382        thrown.expect(IllegalArgumentException.class);
     383        thrown.expectMessage("Illegal option name: '$'");
    319384        new OptionParser("test").addFlagParameter("$", this::nop);
    320385    }
    321386
    322     @Test(expected = IllegalArgumentException.class)
     387    @Test
    323388    public void testDuplicateOptionName() {
     389        thrown.expect(IllegalArgumentException.class);
     390        thrown.expectMessage("The option '--test' is already registered");
    324391        new OptionParser("test").addFlagParameter("test", this::nop).addFlagParameter("test", this::nop);
    325392    }
    326393
    327     @Test(expected = IllegalArgumentException.class)
     394    @Test
    328395    public void testDuplicateOptionName2() {
     396        thrown.expect(IllegalArgumentException.class);
     397        thrown.expectMessage("The option '--test' is already registered");
    329398        new OptionParser("test").addFlagParameter("test", this::nop)
    330399            .addArgumentParameter("test", OptionCount.OPTIONAL, this::nop);
    331400    }
    332401
    333     @Test(expected = IllegalArgumentException.class)
     402    @Test
    334403    public void testInvalidShortAlias() {
     404        thrown.expect(IllegalArgumentException.class);
     405        thrown.expectMessage("Short name '$' must be one character");
    335406        new OptionParser("test").addFlagParameter("test", this::nop).addShortAlias("test", "$");
    336407    }
    337408
    338     @Test(expected = IllegalArgumentException.class)
     409    @Test
    339410    public void testInvalidShortAlias2() {
     411        thrown.expect(IllegalArgumentException.class);
     412        thrown.expectMessage("Short name '' must be one character");
    340413        new OptionParser("test").addFlagParameter("test", this::nop).addShortAlias("test", "");
    341414    }
    342415
    343     @Test(expected = IllegalArgumentException.class)
     416    @Test
    344417    public void testInvalidShortAlias3() {
     418        thrown.expect(IllegalArgumentException.class);
     419        thrown.expectMessage("Short name 'xx' must be one character");
    345420        new OptionParser("test").addFlagParameter("test", this::nop).addShortAlias("test", "xx");
    346421    }
    347422
    348     @Test(expected = IllegalArgumentException.class)
     423    @Test
    349424    public void testDuplicateShortAlias() {
     425        thrown.expect(IllegalArgumentException.class);
     426        thrown.expectMessage("Short name 't' is already used");
    350427        new OptionParser("test").addFlagParameter("test", this::nop)
    351428        .addFlagParameter("test2", this::nop)
    … …  
    354431    }
    355432
    356     @Test(expected = IllegalArgumentException.class)
     433    @Test
    357434    public void testInvalidShortNoLong() {
     435        thrown.expect(IllegalArgumentException.class);
     436        thrown.expectMessage("No long definition for test2 was defined. " +
     437                "Define the long definition first before creating a short definition for it.");
    358438        new OptionParser("test").addFlagParameter("test", this::nop).addShortAlias("test2", "t");
    359439    }
Note: See TracChangeset for help on using the changeset viewer.