From 7575b83437ed8dea5d09726eb248ad50d39a1d0d Mon Sep 17 00:00:00 2001 From: Dan Halperin Date: Wed, 27 Jun 2018 19:25:47 -0700 Subject: [PATCH] CCPE: minor cleanups (#1774) * CCPE: minor cleanups * Replace lambda by method reference * Remove unused parameter * Cleanup building lists * Stringbuilder instead of building string in a loop * Remove or implement some empty if bodies --- .../cisco/CiscoControlPlaneExtractor.java | 94 ++++++++----------- 1 file changed, 38 insertions(+), 56 deletions(-) diff --git a/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java b/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java index 021c8e1f5eb..abdf75b079f 100644 --- a/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java +++ b/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java @@ -1,6 +1,7 @@ package org.batfish.grammar.cisco; import static java.util.Comparator.naturalOrder; +import static java.util.stream.Collectors.toCollection; import static org.batfish.datamodel.ConfigurationFormat.ARISTA; import static org.batfish.datamodel.ConfigurationFormat.ARUBAOS; import static org.batfish.datamodel.ConfigurationFormat.CISCO_ASA; @@ -212,6 +213,7 @@ import java.util.stream.Collectors; import javax.annotation.Nullable; import org.antlr.v4.runtime.ParserRuleContext; +import org.antlr.v4.runtime.RuleContext; import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.tree.ErrorNode; import org.antlr.v4.runtime.tree.ParseTreeWalker; @@ -1164,17 +1166,18 @@ private static int toInteger(Token t) { } private static String toInterfaceName(Interface_nameContext ctx) { - String name = - CiscoConfiguration.getCanonicalInterfaceNamePrefix(ctx.name_prefix_alpha.getText()); + StringBuilder name = + new StringBuilder( + CiscoConfiguration.getCanonicalInterfaceNamePrefix(ctx.name_prefix_alpha.getText())); for (Token part : ctx.name_middle_parts) { - name += part.getText(); + name.append(part.getText()); } if (ctx.range().range_list.size() != 1) { throw new RedFlagBatfishException( "got interface range where single interface was expected: '" + ctx.getText() + "'"); } - name += ctx.range().getText(); - return name; + name.append(ctx.range().getText()); + return name.toString(); } private static long toAsNum(Bgp_asnContext ctx) { @@ -3172,16 +3175,16 @@ public void enterS_interface(S_interfaceContext ctx) { _currentInterfaces = ImmutableList.of(); return; } - String namePrefix = canonicalNamePrefix; + StringBuilder namePrefix = new StringBuilder(canonicalNamePrefix); for (Token part : ctx.iname.name_middle_parts) { - namePrefix += part.getText(); + namePrefix.append(part.getText()); } _currentInterfaces = new ArrayList<>(); if (ctx.iname.range() != null) { List ranges = toRange(ctx.iname.range()); for (SubRange range : ranges) { for (int i = range.getStart(); i <= range.getEnd(); i++) { - String name = namePrefix + i; + String name = namePrefix.toString() + i; addInterface(name, ctx.iname, true); defineStructure(INTERFACE, name, ctx); _configuration.referenceStructure( @@ -3189,7 +3192,7 @@ public void enterS_interface(S_interfaceContext ctx) { } } } else { - addInterface(namePrefix, ctx.iname, true); + addInterface(namePrefix.toString(), ctx.iname, true); } if (ctx.MULTIPOINT() != null) { todo(ctx, F_INTERFACE_MULTIPOINT); @@ -3467,7 +3470,7 @@ public void enterS_username(S_usernameContext ctx) { } else { username = unquote(ctx.quoted_user.getText()); } - _currentUser = _configuration.getCf().getUsers().computeIfAbsent(username, k -> new User(k)); + _currentUser = _configuration.getCf().getUsers().computeIfAbsent(username, User::new); } @Override @@ -3630,7 +3633,8 @@ public void exitAaa_accounting_commands_line(Aaa_accounting_commands_lineContext @Override public void exitAaa_accounting_default_group(Aaa_accounting_default_groupContext ctx) { - List groups = ctx.groups.stream().map(g -> g.getText()).collect(Collectors.toList()); + List groups = + ctx.groups.stream().map(RuleContext::getText).collect(Collectors.toList()); _configuration.getCf().getAaa().getAccounting().getDefault().setGroups(groups); } @@ -3908,7 +3912,7 @@ private MatchSemantics toMatchSemantics(Match_semanticsContext ctx) { @Override public void exitS_zone(S_zoneContext ctx) { String name = ctx.name.getText(); - _configuration.getSecurityZones().computeIfAbsent(name, n -> new SecurityZone(n)); + _configuration.getSecurityZones().computeIfAbsent(name, SecurityZone::new); defineStructure(SECURITY_ZONE, name, ctx); } @@ -4316,9 +4320,9 @@ private AccessListServiceSpecifier computeExtendedAccessListServiceSpecifier( if (ctx.prot != null) { IpProtocol protocol = toIpProtocol(ctx.prot); List srcPortRanges = - ctx.alps_src != null ? toPortRanges(ctx.alps_src) : Collections.emptyList(); + ctx.alps_src != null ? toPortRanges(ctx.alps_src) : Collections.emptyList(); List dstPortRanges = - ctx.alps_dst != null ? toPortRanges(ctx.alps_dst) : Collections.emptyList(); + ctx.alps_dst != null ? toPortRanges(ctx.alps_dst) : Collections.emptyList(); Integer icmpType = null; Integer icmpCode = null; List tcpFlags = new ArrayList<>(); @@ -4532,9 +4536,9 @@ public void exitExtended_ipv6_access_list_tail(Extended_ipv6_access_list_tailCon String srcAddressGroup = getAddressGroup(ctx.srcipr); String dstAddressGroup = getAddressGroup(ctx.dstipr); List srcPortRanges = - ctx.alps_src != null ? toPortRanges(ctx.alps_src) : Collections.emptyList(); + ctx.alps_src != null ? toPortRanges(ctx.alps_src) : Collections.emptyList(); List dstPortRanges = - ctx.alps_dst != null ? toPortRanges(ctx.alps_dst) : Collections.emptyList(); + ctx.alps_dst != null ? toPortRanges(ctx.alps_dst) : Collections.emptyList(); Integer icmpType = null; Integer icmpCode = null; List tcpFlags = new ArrayList<>(); @@ -5424,11 +5428,11 @@ public void exitIp_community_list_expanded_stanza(Ip_community_list_expanded_sta @Override public void exitIp_community_list_expanded_tail(Ip_community_list_expanded_tailContext ctx) { LineAction action = toLineAction(ctx.ala); - String regex = ""; + StringBuilder regex = new StringBuilder(); for (Token remainder : ctx.remainder) { - regex += remainder.getText(); + regex.append(remainder.getText()); } - ExpandedCommunityListLine line = new ExpandedCommunityListLine(action, regex); + ExpandedCommunityListLine line = new ExpandedCommunityListLine(action, regex.toString()); _currentExpandedCommunityList.addLine(line); } @@ -5695,7 +5699,7 @@ public void exitL_login_authentication(L_login_authenticationContext ctx) { @Override public void exitL_transport(L_transportContext ctx) { SortedSet protocols = - new TreeSet<>(ctx.prot.stream().map(c -> c.getText()).collect(Collectors.toSet())); + ctx.prot.stream().map(RuleContext::getText).collect(toCollection(TreeSet::new)); BiConsumer> setter; if (ctx.INPUT() != null) { setter = Line::setTransportInput; @@ -5998,8 +6002,6 @@ public void exitNeighbor_block_rb_stanza(Neighbor_block_rb_stanzaContext ctx) { resetPeerGroups(); if (_inBlockNeighbor) { popPeer(); - } else { - _inBlockNeighbor = false; } } @@ -6350,7 +6352,7 @@ public void exitPim_spt_threshold(Pim_spt_thresholdContext ctx) { public void enterPm_ios_inspect(Pm_ios_inspectContext ctx) { String name = ctx.name.getText(); _currentInspectPolicyMap = - _configuration.getInspectPolicyMaps().computeIfAbsent(name, n -> new InspectPolicyMap(n)); + _configuration.getInspectPolicyMaps().computeIfAbsent(name, InspectPolicyMap::new); defineStructure(INSPECT_POLICY_MAP, name, ctx); } @@ -7192,7 +7194,7 @@ public void exitS_domain_name(S_domain_nameContext ctx) { @Override public void exitS_feature(S_featureContext ctx) { - List words = ctx.words.stream().map(w -> w.getText()).collect(Collectors.toList()); + List words = ctx.words.stream().map(RuleContext::getText).collect(Collectors.toList()); boolean enabled = ctx.NO() == null; String name = String.join(".", words); _configuration.getCf().getFeatures().put(name, enabled); @@ -7303,7 +7305,7 @@ public void exitS_router_rip(S_router_ripContext ctx) { @Override public void exitS_service(S_serviceContext ctx) { - List words = ctx.words.stream().map(w -> w.getText()).collect(Collectors.toList()); + List words = ctx.words.stream().map(RuleContext::getText).collect(Collectors.toList()); boolean enabled = ctx.NO() == null; Iterator i = words.iterator(); SortedMap currentServices = _configuration.getCf().getServices(); @@ -7312,7 +7314,7 @@ public void exitS_service(S_serviceContext ctx) { Service s = currentServices.computeIfAbsent(name, k -> new Service()); if (enabled) { s.setEnabled(true); - } else if (!enabled && !i.hasNext()) { + } else if (!i.hasNext()) { s.disable(); } currentServices = s.getSubservices(); @@ -7579,7 +7581,7 @@ public void exitSs_enable_traps(Ss_enable_trapsContext ctx) { if (ctx.snmp_trap_type != null) { String trapName = ctx.snmp_trap_type.getText(); SortedSet subfeatureNames = - new TreeSet<>(ctx.subfeature.stream().map(s -> s.getText()).collect(Collectors.toList())); + ctx.subfeature.stream().map(RuleContext::getText).collect(toCollection(TreeSet::new)); SortedMap> traps = _configuration.getSnmpServer().getTraps(); SortedSet subfeatures = traps.get(trapName); if (subfeatures == null) { @@ -7878,9 +7880,7 @@ public void exitUpdate_source_bgp_tail(Update_source_bgp_tailContext ctx) { _configuration.referenceStructure( INTERFACE, source, BGP_UPDATE_SOURCE_INTERFACE, ctx.getStart().getLine()); - if (_currentPeerGroup == null) { - return; - } else { + if (_currentPeerGroup != null) { _currentPeerGroup.setUpdateSource(source); } } @@ -8145,7 +8145,7 @@ private AsPathSetElem toAsPathSetElem(As_path_set_elemContext ctx) { private AsPathSetExpr toAsPathSetExpr(As_path_set_inlineContext ctx) { List elems = - ctx.elems.stream().map(e -> toAsPathSetElem(e)).collect(Collectors.toList()); + ctx.elems.stream().map(this::toAsPathSetElem).collect(Collectors.toList()); return new ExplicitAsPathSet(elems); } @@ -8929,17 +8929,13 @@ private RoutePolicyBoolean toRoutePolicyBoolean(Boolean_as_path_in_rp_stanzaCont return new RoutePolicyBooleanAsPathIn(asPathSetExpr); } - private RoutePolicyBoolean toRoutePolicyBoolean(Boolean_as_path_is_local_rp_stanzaContext ctx) { - return new RoutePolicyBooleanAsPathIsLocal(); - } - private RoutePolicyBoolean toRoutePolicyBoolean( Boolean_as_path_neighbor_is_rp_stanzaContext ctx) { List range = ctx.as_range_expr() .subranges .stream() - .map(sr -> toSubRangeExpr(sr)) + .map(this::toSubRangeExpr) .collect(Collectors.toList()); boolean exact = ctx.as_range_expr().EXACT() != null; return new RoutePolicyBooleanAsPathNeighborIs(range, exact); @@ -8951,7 +8947,7 @@ private RoutePolicyBoolean toRoutePolicyBoolean( ctx.as_range_expr() .subranges .stream() - .map(sr -> toSubRangeExpr(sr)) + .map(this::toSubRangeExpr) .collect(Collectors.toList()); boolean exact = ctx.as_range_expr().EXACT() != null; return new RoutePolicyBooleanAsPathOriginatesFrom(range, exact); @@ -8963,7 +8959,7 @@ private RoutePolicyBoolean toRoutePolicyBoolean( ctx.as_range_expr() .subranges .stream() - .map(sr -> toSubRangeExpr(sr)) + .map(this::toSubRangeExpr) .collect(Collectors.toList()); boolean exact = ctx.as_range_expr().EXACT() != null; return new RoutePolicyBooleanAsPathPassesThrough(range, exact); @@ -9048,7 +9044,7 @@ private RoutePolicyBoolean toRoutePolicyBoolean(Boolean_simple_rp_stanzaContext Boolean_as_path_is_local_rp_stanzaContext alctx = ctx.boolean_as_path_is_local_rp_stanza(); if (alctx != null) { - return toRoutePolicyBoolean(alctx); + return new RoutePolicyBooleanAsPathIsLocal(); } Boolean_as_path_neighbor_is_rp_stanzaContext anctx = @@ -9132,10 +9128,7 @@ private RoutePolicyCommunitySet toRoutePolicyCommunitySet(Rp_community_setContex } else { // inline return new RoutePolicyCommunitySetInline( - ctx.elems - .stream() - .map(elem -> toCommunitySetElemExpr(elem)) - .collect(Collectors.toList())); + ctx.elems.stream().map(this::toCommunitySetElemExpr).collect(Collectors.toList())); } } @@ -9352,10 +9345,6 @@ private RoutePolicyStatement toRoutePolicyStatement(Set_next_hop_rp_stanzaContex return new RoutePolicySetNextHop(hop, destVrf); } - private RoutePolicyStatement toRoutePolicyStatement(Set_next_hop_self_rp_stanzaContext ctx) { - return new RoutePolicySetNextHop(new RoutePolicyNextHopSelf(), false); - } - private RoutePolicyStatement toRoutePolicyStatement(Set_origin_rp_stanzaContext ctx) { OriginExpr origin = toOriginExpr(ctx.origin_expr()); return new RoutePolicySetOrigin(origin); @@ -9404,7 +9393,7 @@ private RoutePolicyStatement toRoutePolicyStatement(Set_rp_stanzaContext ctx) { Set_next_hop_self_rp_stanzaContext nhsctx = ctx.set_next_hop_self_rp_stanza(); if (nhsctx != null) { - return toRoutePolicyStatement(nhsctx); + return new RoutePolicySetNextHop(new RoutePolicyNextHopSelf(), false); } Set_origin_rp_stanzaContext octx = ctx.set_origin_rp_stanza(); @@ -9436,14 +9425,7 @@ private RoutePolicyStatement toRoutePolicyStatement(Set_weight_rp_stanzaContext } private List toRoutePolicyStatementList(List ctxts) { - List stmts = new ArrayList<>(); - for (Rp_stanzaContext ctxt : ctxts) { - RoutePolicyStatement stmt = toRoutePolicyStatement(ctxt); - if (stmt != null) { - stmts.add(stmt); - } - } - return stmts; + return ctxts.stream().map(this::toRoutePolicyStatement).collect(Collectors.toList()); } private RouteTypeExpr toRouteType(Rp_route_typeContext ctx) {