From 0ad3c0ff6ff895ac40240494e7036e0ad7d50126 Mon Sep 17 00:00:00 2001 From: Andrey Turbanov Date: Tue, 17 May 2022 09:43:28 +0300 Subject: [PATCH 1/2] Avoid redundant Map.containsKey call --- .../main/java/org/jline/builtins/Options.java | 13 +++---- .../main/java/org/jline/builtins/Styles.java | 3 -- .../org/jline/builtins/SyntaxHighlighter.java | 15 ++++--- .../console/impl/AbstractCommandRegistry.java | 11 +----- .../jline/console/impl/ConsoleEngineImpl.java | 4 +- .../jline/console/impl/DefaultPrinter.java | 39 ++++++++++--------- .../console/impl/SystemRegistryImpl.java | 21 +++++----- .../java/org/jline/widget/TailTipWidgets.java | 7 ++-- .../test/java/org/jline/example/Console.java | 4 +- .../impl/completer/SystemCompleter.java | 2 +- 10 files changed, 56 insertions(+), 63 deletions(-) diff --git a/builtins/src/main/java/org/jline/builtins/Options.java b/builtins/src/main/java/org/jline/builtins/Options.java index a51266739..88249c363 100644 --- a/builtins/src/main/java/org/jline/builtins/Options.java +++ b/builtins/src/main/java/org/jline/builtins/Options.java @@ -127,10 +127,11 @@ public Options setOptionsFirst(boolean optionsFirst) { } public boolean isSet(String name) { - if (!optSet.containsKey(name)) + Boolean isSet = optSet.get(name); + if (isSet == null) { throw new IllegalArgumentException("option not defined in spec: " + name); - - return optSet.get(name); + } + return isSet; } public Object getObject(String name) { @@ -311,9 +312,8 @@ private void parseSpec(Map myOptSet, Map myOptA final String name = (opt != null) ? opt : m.group(GROUP_SHORT_OPT_1); if (name != null) { - if (myOptSet.containsKey(name)) + if (myOptSet.putIfAbsent(name, false) != null) throw new IllegalArgumentException("duplicate option in spec: --" + name); - myOptSet.put(name, false); } String dflt = (m.group(GROUP_DEFAULT) != null) ? m.group(GROUP_DEFAULT) : ""; @@ -331,9 +331,8 @@ private void parseSpec(Map myOptSet, Map myOptA for (int i = 0; i < 2; ++i) { String sopt = m.group(i == 0 ? GROUP_SHORT_OPT_1 : GROUP_SHORT_OPT_2); if (sopt != null) { - if (optName.containsKey(sopt)) + if (optName.putIfAbsent(sopt, name) != null) throw new IllegalArgumentException("duplicate option in spec: -" + sopt); - optName.put(sopt, name); } } } diff --git a/builtins/src/main/java/org/jline/builtins/Styles.java b/builtins/src/main/java/org/jline/builtins/Styles.java index becb8283f..5dd006f83 100644 --- a/builtins/src/main/java/org/jline/builtins/Styles.java +++ b/builtins/src/main/java/org/jline/builtins/Styles.java @@ -132,9 +132,6 @@ public StyleCompiler(Map colors, boolean nanoStyle) { } public String getStyle(String reference) { - if (!colors.containsKey(reference)) { - return null; - } String rawStyle = colors.get(reference); if (rawStyle == null) { return null; diff --git a/builtins/src/main/java/org/jline/builtins/SyntaxHighlighter.java b/builtins/src/main/java/org/jline/builtins/SyntaxHighlighter.java index dafe6a986..ae315cdf0 100644 --- a/builtins/src/main/java/org/jline/builtins/SyntaxHighlighter.java +++ b/builtins/src/main/java/org/jline/builtins/SyntaxHighlighter.java @@ -543,8 +543,9 @@ public void parse() throws IOException { } } else if (!addHighlightRule(parts, idx, TOKEN_NANORC) && parts.get(0).matches("\\+" + REGEX_TOKEN_NAME)) { String key = themeKey(parts.get(0)); - if (colorTheme.containsKey(key)) { - for (String l : colorTheme.get(key).split("\\\\n")) { + String theme = colorTheme.get(key); + if (theme != null) { + for (String l : theme.split("\\\\n")) { idx++; addHighlightRule(RuleSplitter.split(fixRegexes(l)), idx, TOKEN_NANORC); } @@ -584,18 +585,20 @@ private boolean addHighlightRule(List parts, int idx, String tokenName) addHighlightRule(syntaxName + idx, parts, true, tokenName); } else if (parts.get(0).matches(REGEX_TOKEN_NAME + "[:]?")) { String key = themeKey(parts.get(0)); - if (colorTheme.containsKey(key)) { + String theme = colorTheme.get(key); + if (theme != null) { parts.set(0, "color"); - parts.add(1, colorTheme.get(key)); + parts.add(1, theme); addHighlightRule(syntaxName + idx, parts, false, tokenName); } else { Log.warn("Unknown token type: ", key); } } else if (parts.get(0).matches("~" + REGEX_TOKEN_NAME + "[:]?")) { String key = themeKey(parts.get(0)); - if (colorTheme.containsKey(key)) { + String theme = colorTheme.get(key); + if (theme != null) { parts.set(0, "icolor"); - parts.add(1, colorTheme.get(key)); + parts.add(1, theme); addHighlightRule(syntaxName + idx, parts, true, tokenName); } else { Log.warn("Unknown token type: ", key); diff --git a/console/src/main/java/org/jline/console/impl/AbstractCommandRegistry.java b/console/src/main/java/org/jline/console/impl/AbstractCommandRegistry.java index 08306c851..04907519c 100644 --- a/console/src/main/java/org/jline/console/impl/AbstractCommandRegistry.java +++ b/console/src/main/java/org/jline/console/impl/AbstractCommandRegistry.java @@ -185,12 +185,7 @@ public SystemCompleter compileCompleters() { public T command(String name) { T out; - if (!hasCommand(name)) { - throw new IllegalArgumentException("Command does not exists!"); - } - if (aliasCommand.containsKey(name)) { - name = aliasCommand.get(name); - } + name = aliasCommand.getOrDefault(name, name); if (nameCommand.containsKey(name)) { out = nameCommand.get(name); } else { @@ -248,10 +243,8 @@ public SystemCompleter compileCompleters() { public String command(String name) { if (commandExecute.containsKey(name)) { return name; - } else if (aliasCommand.containsKey(name)) { - return aliasCommand.get(name); } - return null; + return aliasCommand.get(name); } public CommandMethods getCommandMethods(String command) { diff --git a/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java b/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java index 3056dbb1e..f2949c5a2 100644 --- a/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java +++ b/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java @@ -1283,8 +1283,8 @@ public void complete(LineReader reader, ParsedLine commandLine, List if (words.size() > 1) { String h = words.get(words.size() - 2); if (h != null && h.length() > 0) { - if(aliases.containsKey(h)){ - String v = aliases.get(h); + String v = aliases.get(h); + if (v != null){ candidates.add(new Candidate(AttributedString.stripAnsi(v) , v, null, null, null, null, true)); } diff --git a/console/src/main/java/org/jline/console/impl/DefaultPrinter.java b/console/src/main/java/org/jline/console/impl/DefaultPrinter.java index 362eac4db..0e1cae5cd 100644 --- a/console/src/main/java/org/jline/console/impl/DefaultPrinter.java +++ b/console/src/main/java/org/jline/console/impl/DefaultPrinter.java @@ -273,11 +273,10 @@ protected Terminal terminal() { */ protected void manageBooleanOptions(Map options) { for (String key : Printer.BOOLEAN_KEYS) { - if (options.containsKey(key)) { - boolean value = options.get(key) instanceof Boolean && (boolean) options.get(key); - if (!value) { - options.remove(key); - } + Object option = options.get(key); + boolean value = option instanceof Boolean && (boolean)option; + if (!value) { + options.remove(key); } } } @@ -512,15 +511,17 @@ private Object mapValue(Map options, String key, Map optionList(String key, Map options) { List out = new ArrayList<>(); - if (options.containsKey(key)) { - if (options.get(key) instanceof String) { - out.addAll(Arrays.asList(((String)options.get(key)).split(","))); - } else if (options.get(key) instanceof Collection) { - out.addAll((Collection)options.get(key)); - } else { - throw new IllegalArgumentException("Unsupported option list: {key: " + key - + ", type: " + options.get(key).getClass() + "}"); - } + Object option = options.get(key); + if (option == null) { + return out; + } + if (option instanceof String) { + out.addAll(Arrays.asList(((String)option).split(","))); + } else if (option instanceof Collection) { + out.addAll((Collection)option); + } else { + throw new IllegalArgumentException("Unsupported option list: {key: " + key + + ", type: " + option.getClass() + "}"); } return out; } @@ -559,9 +560,8 @@ private String columnValue(String value) { @SuppressWarnings("unchecked") private Map objectToMap(Map options, Object obj) { if (obj != null) { - Map, Object> toMap = options.containsKey(Printer.OBJECT_TO_MAP) - ? (Map, Object>)options.get(Printer.OBJECT_TO_MAP) - : new HashMap<>(); + Map, Object> toMap = (Map, Object>) + options.getOrDefault(Printer.OBJECT_TO_MAP, Collections.emptyMap()); if (toMap.containsKey(obj.getClass())) { return (Map)engine.execute(toMap.get(obj.getClass()), obj); } else if (objectToMap.containsKey(obj.getClass())) { @@ -641,8 +641,9 @@ private AttributedString highlightValue(Map options, String colu if (hv.containsKey("*")) { out = (AttributedString) engine.execute(hv.get("*"), out); } - if (highlightValue.containsKey("*")) { - out = highlightValue.get("*").apply(out); + Function func = highlightValue.get("*"); + if (func != null) { + out = func.apply(out); } } if (options.containsKey(Printer.VALUE_STYLE) && !isHighlighted(out)) { diff --git a/console/src/main/java/org/jline/console/impl/SystemRegistryImpl.java b/console/src/main/java/org/jline/console/impl/SystemRegistryImpl.java index 4c476a5ca..5640057eb 100644 --- a/console/src/main/java/org/jline/console/impl/SystemRegistryImpl.java +++ b/console/src/main/java/org/jline/console/impl/SystemRegistryImpl.java @@ -191,8 +191,9 @@ public void register(String command, CommandRegistry subcommandRegistry) { private List localCommandInfo(String command) { try { - if (subcommands.containsKey(command)) { - registryHelp(subcommands.get(command)); + CommandRegistry subCommand = subcommands.get(command); + if (subCommand != null) { + registryHelp(subCommand); } else { localExecute(command, new String[] { "--help" }); } @@ -275,8 +276,9 @@ private SystemCompleter _compileCompleters() { SystemCompleter out = CommandRegistry.aggregateCompleters(commandRegistries); SystemCompleter local = new SystemCompleter(); for (String command : commandExecute.keySet()) { - if (subcommands.containsKey(command)) { - for(Map.Entry> entry : subcommands.get(command).compileCompleters().getCompleters().entrySet()) { + CommandRegistry subCommand = subcommands.get(command); + if (subCommand != null) { + for (Map.Entry> entry : subCommand.compileCompleters().getCompleters().entrySet()) { for (Completer cc : entry.getValue()) { if (!(cc instanceof ArgumentCompleter)) { throw new IllegalArgumentException(); @@ -369,18 +371,19 @@ public CmdDesc commandDescription(CmdLine line) { case COMMAND: if (isCommandOrScript(cmd) && !names.hasPipes(line.getArgs())) { List args = line.getArgs(); - if (subcommands.containsKey(cmd)) { + CommandRegistry subCommand = subcommands.get(cmd); + if (subCommand != null) { String c = args.size() > 1 ? args.get(1) : null; - if (c == null || subcommands.get(cmd).hasCommand(c)) { + if (c == null || subCommand.hasCommand(c)) { if (c != null && c.equals("help")) { out = null; } else if (c != null) { - out = subcommands.get(cmd).commandDescription(Collections.singletonList(c)); + out = subCommand.commandDescription(Collections.singletonList(c)); } else { - out = commandDescription(subcommands.get(cmd)); + out = commandDescription(subCommand); } } else { - out = commandDescription(subcommands.get(cmd)); + out = commandDescription(subCommand); } if (out != null) { out.setSubcommand(true); diff --git a/console/src/main/java/org/jline/widget/TailTipWidgets.java b/console/src/main/java/org/jline/widget/TailTipWidgets.java index 03bc6f4eb..abd96edd9 100644 --- a/console/src/main/java/org/jline/widget/TailTipWidgets.java +++ b/console/src/main/java/org/jline/widget/TailTipWidgets.java @@ -744,14 +744,13 @@ private Pair evaluateCommandLine(String line, List args, } public CmdDesc getDescription(String command) { - CmdDesc out = null; + CmdDesc out; if (descriptions.containsKey(command)) { out = descriptions.get(command); } else if (temporaryDescs.containsKey(command)) { out = temporaryDescs.get(command); - } else if (volatileDescs.containsKey(command)) { - out = volatileDescs.get(command); - volatileDescs.remove(command); + } else { + out = volatileDescs.remove(command); } return out; } diff --git a/console/src/test/java/org/jline/example/Console.java b/console/src/test/java/org/jline/example/Console.java index 0f782b13c..d6a20c9e1 100644 --- a/console/src/test/java/org/jline/example/Console.java +++ b/console/src/test/java/org/jline/example/Console.java @@ -153,10 +153,8 @@ public boolean hasCommand(String command) { private String command(String name) { if (commandExecute.containsKey(name)) { return name; - } else if (aliasCommand.containsKey(name)) { - return aliasCommand.get(name); } - return null; + return aliasCommand.get(name); } public SystemCompleter compileCompleters() { diff --git a/reader/src/main/java/org/jline/reader/impl/completer/SystemCompleter.java b/reader/src/main/java/org/jline/reader/impl/completer/SystemCompleter.java index 0484da330..a9356fc38 100644 --- a/reader/src/main/java/org/jline/reader/impl/completer/SystemCompleter.java +++ b/reader/src/main/java/org/jline/reader/impl/completer/SystemCompleter.java @@ -67,7 +67,7 @@ private String command(String cmd) { if (cmd != null) { if (completers.containsKey(cmd)) { out = cmd; - } else if (aliasCommand.containsKey(cmd)) { + } else { out = aliasCommand.get(cmd); } } From edab6c67170655113b1df283fb16ff030311fce7 Mon Sep 17 00:00:00 2001 From: Andrey Turbanov Date: Tue, 17 May 2022 19:40:14 +0300 Subject: [PATCH 2/2] Avoid redundant Map.containsKey call. Fix review comments --- .../jline/console/impl/ConsoleEngineImpl.java | 2 +- .../org/jline/console/impl/DefaultPrinter.java | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java b/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java index f2949c5a2..723859c1b 100644 --- a/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java +++ b/console/src/main/java/org/jline/console/impl/ConsoleEngineImpl.java @@ -1284,7 +1284,7 @@ public void complete(LineReader reader, ParsedLine commandLine, List String h = words.get(words.size() - 2); if (h != null && h.length() > 0) { String v = aliases.get(h); - if (v != null){ + if (v != null) { candidates.add(new Candidate(AttributedString.stripAnsi(v) , v, null, null, null, null, true)); } diff --git a/console/src/main/java/org/jline/console/impl/DefaultPrinter.java b/console/src/main/java/org/jline/console/impl/DefaultPrinter.java index 0e1cae5cd..15d5882ee 100644 --- a/console/src/main/java/org/jline/console/impl/DefaultPrinter.java +++ b/console/src/main/java/org/jline/console/impl/DefaultPrinter.java @@ -274,7 +274,7 @@ protected Terminal terminal() { protected void manageBooleanOptions(Map options) { for (String key : Printer.BOOLEAN_KEYS) { Object option = options.get(key); - boolean value = option instanceof Boolean && (boolean)option; + boolean value = option instanceof Boolean && (boolean) option; if (!value) { options.remove(key); } @@ -512,14 +512,11 @@ private Object mapValue(Map options, String key, Map optionList(String key, Map options) { List out = new ArrayList<>(); Object option = options.get(key); - if (option == null) { - return out; - } if (option instanceof String) { out.addAll(Arrays.asList(((String)option).split(","))); } else if (option instanceof Collection) { out.addAll((Collection)option); - } else { + } else if (option != null) { throw new IllegalArgumentException("Unsupported option list: {key: " + key + ", type: " + option.getClass() + "}"); } @@ -635,14 +632,14 @@ private AttributedString highlightValue(Map options, String colu } else { out = new AttributedString(columnValue(objectToString(options, raw))); } - } - if ((simpleObject(raw) || raw == null) && (hv.containsKey("*") || highlightValue.containsKey("*")) + } + if ((simpleObject(raw) || raw == null) && (hv.containsKey("*") || highlightValue.containsKey("*")) && !isHighlighted(out)) { if (hv.containsKey("*")) { - out = (AttributedString) engine.execute(hv.get("*"), out); + out = (AttributedString)engine.execute(hv.get("*"), out); } - Function func = highlightValue.get("*"); - if (func != null) { + Function func = highlightValue.get("*"); + if (func != null) { out = func.apply(out); } }