Merge pull request #689 from scireum/aha/sonarlint

Aha/sonarlint
scireum · Dec 22, 2019 · 83b1716 · 83b1716
2 parents b49c0ff + a8b39a3
commit 83b1716
Show file tree

Hide file tree

Showing 19 changed files with 139 additions and 216 deletions.
diff --git a/pom.xml b/pom.xml
@@ -14,7 +14,7 @@
     <description>Provides a modern and scalable web server as SIRIUS module</description>
 
     <properties>
-        <sirius.kernel>13.20</sirius.kernel>
+        <sirius.kernel>14.0</sirius.kernel>
     </properties>
 
     <repositories>

diff --git a/src/main/java/sirius/tagliatelle/compiler/Compiler.java b/src/main/java/sirius/tagliatelle/compiler/Compiler.java
@@ -130,7 +130,6 @@ private List<CompileError> processCollectedErrors() throws CompileException {
         return compileErrors;
     }
 
-
     /**
      * Verifies all macro calls to ensure their integrity.
      */
@@ -148,7 +147,7 @@ private void verifyMacros() {
     private Expression verifyMacro(Position pos, Expression expr) {
         if (expr instanceof MacroCall) {
             try {
-                ((MacroCall) expr).verify(context,pos);
+                ((MacroCall) expr).verify(context, pos);
             } catch (IllegalArgumentException ex) {
                 context.error(pos, "Invalid parameters for macro: %s: %s", expr, ex.getMessage());
             }
@@ -195,11 +194,10 @@ protected CompositeEmitter parseBlock(@Nullable TagHandler parentHandler, @Nulla
                 return block;
             }
 
-            if (processTag(parentHandler, block, staticText) || processExpression(block)) {
-                if (!reader.current().isEndOfInput()) {
-                    staticText = new ConstantEmitter(reader.current());
-                    block.addChild(staticText);
-                }
+            if ((processTag(parentHandler, block, staticText) || processExpression(block)) && !reader.current()
+                                                                                                     .isEndOfInput()) {
+                staticText = new ConstantEmitter(reader.current());
+                block.addChild(staticText);
             }
         }
 
@@ -405,7 +403,7 @@ private void handleTag(TagHandler handler, CompositeEmitter block) {
     private void parseAttributes(TagHandler handler) {
         while (true) {
             skipWhitespaces();
-            if (reader.current().isEndOfInput() || reader.current().is('>','/')) {
+            if (reader.current().isEndOfInput() || reader.current().is('>', '/')) {
                 break;
             }
             String name = parseName();

diff --git a/src/main/java/sirius/tagliatelle/compiler/Parser.java b/src/main/java/sirius/tagliatelle/compiler/Parser.java
@@ -454,11 +454,12 @@ private Expression handleSpecialMethods(Expression self, String methodName, List
             }
         }
 
-        if ("is".equals(methodName) && parameters.size() == 1 && (parameters.get(0) instanceof ConstantClass)) {
-            if (!Transformable.class.isAssignableFrom(self.getType())) {
-                Class<?> type = (Class<?>) parameters.get(0).eval(null);
-                return new InstanceCheck(self, type);
-            }
+        if ("is".equals(methodName)
+            && parameters.size() == 1
+            && (parameters.get(0) instanceof ConstantClass)
+            && !Transformable.class.isAssignableFrom(self.getType())) {
+            Class<?> type = (Class<?>) parameters.get(0).eval(null);
+            return new InstanceCheck(self, type);
         }
 
         return null;

diff --git a/src/main/java/sirius/tagliatelle/expression/MethodCall.java b/src/main/java/sirius/tagliatelle/expression/MethodCall.java
@@ -9,6 +9,7 @@
 package sirius.tagliatelle.expression;
 
 import parsii.tokenizer.Char;
+import sirius.kernel.commons.Explain;
 import sirius.kernel.commons.Strings;
 import sirius.kernel.health.Exceptions;
 import sirius.tagliatelle.Tagliatelle;
@@ -253,10 +254,8 @@ private Method findMethod(Class<?> type, String name, Class<?>[] parameterTypes)
 
         // Try to find an appropriate method using coercions known to the system...
         for (Method m : type.getMethods()) {
-            if (signatureMatch(m, name, parameterTypes)) {
-                if (checkSandbox(m)) {
-                    return m;
-                }
+            if (signatureMatch(m, name, parameterTypes) && checkSandbox(m)) {
+                return m;
             }
         }
 
@@ -299,38 +298,51 @@ private boolean signatureMatch(Method method, String name, Class<?>[] parameterT
         return ensureEnoughParameters(method, parameterTypes);
     }
 
+    @SuppressWarnings("squid:S3776")
+    @Explain("As this involves a stateful complex check, we rather keep all checks in one place.")
     private boolean checkParameterTypes(Method method, Class<?>[] parameterTypes) {
         Class<?> varargType = null;
         for (int i = 0; i < parameterTypes.length; i++) {
             Class<?> parameterType = parameterTypes[i];
-            if (i == method.getParameterCount() - 1 && method.getParameterTypes()[i].isArray()) {
-                varargType = method.getParameterTypes()[i].getComponentType();
-            }
-            if (i >= method.getParameterCount() || !Tagliatelle.isAssignableTo(parameterType,
-                                                                               method.getParameterTypes()[i])) {
-                if (varargType == null || !Tagliatelle.isAssignableTo(parameterType, varargType)) {
+            Class<?> methodParameterType = i < method.getParameterCount() ? method.getParameterTypes()[i] : varargType;
+
+            // If the last parameter is an array, we have to determine its component type as this might be
+            // a var-arg call...
+            if (method.isVarArgs() && i == method.getParameterCount() - 1 && methodParameterType.isArray()) {
+                varargType = methodParameterType.getComponentType();
+
+                // For a var-arg parameter, the last parameter has either to be the exact same type (an array)
+                // or it has to match the vararg type
+                if (!Tagliatelle.isAssignableTo(parameterType, methodParameterType) && !Tagliatelle.isAssignableTo(
+                        parameterType,
+                        varargType)) {
                     return false;
                 }
+
+                // If we matched the original array type, we cannot support additional vararg parameters,
+                // therefore the argument count must match...
+                if (!Tagliatelle.isAssignableTo(parameterType, varargType)
+                    && parameterTypes.length > method.getParameterCount()) {
+                    return false;
+                }
+            } else if (methodParameterType == null || !Tagliatelle.isAssignableTo(parameterType, methodParameterType)) {
+                // For all other parameters (than the last) we simply ensure, that the parameter type is assignable...
+                return false;
             }
         }
+
         return true;
     }
 
     private boolean ensureEnoughParameters(Method method, Class<?>[] parameterTypes) {
         // The method accepts all given parameters, now ensure, that we also provide enough parameters for the method...
         if (!method.isVarArgs()) {
             // No varargs -> parameters must match exactly...
-            if (method.getParameterTypes().length != parameterTypes.length) {
-                return false;
-            }
+            return method.getParameterTypes().length == parameterTypes.length;
         } else {
             // Varary -> we can at most skip the last parameter...
-            if (parameterTypes.length < method.getParameterTypes().length - 1) {
-                return false;
-            }
+            return parameterTypes.length >= method.getParameterTypes().length - 1;
         }
-
-        return true;
     }
 
     @Override

diff --git a/src/main/java/sirius/web/controller/ControllerDispatcher.java b/src/main/java/sirius/web/controller/ControllerDispatcher.java
@@ -121,13 +121,13 @@ private void preparePerformRoute(WebContext ctx,
                                      List<Object> params,
                                      InputStreamHandler inputStreamHandler) {
         try {
-            if (firewall != null && !route.getMethod().isAnnotationPresent(Unlimited.class)) {
-                if (firewall.handleRateLimiting(ctx,
-                                                Optional.ofNullable(route.getMethod().getAnnotation(Limited.class))
-                                                        .map(Limited::value)
-                                                        .orElse(Limited.HTTP))) {
-                    return;
-                }
+            if (firewall != null
+                && !route.getMethod().isAnnotationPresent(Unlimited.class)
+                && firewall.handleRateLimiting(ctx,
+                                               Optional.ofNullable(route.getMethod().getAnnotation(Limited.class))
+                                                       .map(Limited::value)
+                                                       .orElse(Limited.HTTP))) {
+                return;
             }
 
             // Inject WebContext as first parameter...

diff --git a/src/main/java/sirius/web/controller/Page.java b/src/main/java/sirius/web/controller/Page.java
@@ -11,7 +11,6 @@
 import com.google.common.collect.Lists;
 import sirius.kernel.cache.ValueComputer;
 import sirius.kernel.commons.Limit;
-import sirius.kernel.commons.Monoflop;
 import sirius.kernel.commons.Strings;
 import sirius.kernel.nls.NLS;
 import sirius.web.http.WebContext;
@@ -449,62 +448,4 @@ public boolean hasFacets() {
     public int getPageSize() {
         return pageSize;
     }
-
-    private boolean addQueryToQueryString(String field,
-                                          String value,
-                                          StringBuilder queryStringBuilder,
-                                          Monoflop ampersandPlaced) {
-        if (PARAM_QUERY.equals(field)) {
-            queryStringBuilder.append(ampersandPlaced.firstCall() ? "" : "&");
-            queryStringBuilder.append("query=");
-            queryStringBuilder.append(Strings.urlEncode(value));
-            return true;
-        }
-
-        if (Strings.isFilled(query)) {
-            queryStringBuilder.append(ampersandPlaced.firstCall() ? "" : "&");
-            queryStringBuilder.append("query=");
-            queryStringBuilder.append(Strings.urlEncode(query));
-        }
-        return false;
-    }
-
-    private boolean addStartToQueryString(String field,
-                                          String value,
-                                          StringBuilder queryStringBuilder,
-                                          Monoflop ampersandPlaced) {
-        queryStringBuilder.append(ampersandPlaced.firstCall() ? "" : "&");
-        queryStringBuilder.append("start=");
-        if (PARAM_START.equals(field)) {
-            queryStringBuilder.append(value);
-            return true;
-        } else {
-            queryStringBuilder.append(start);
-            return false;
-        }
-    }
-
-    private boolean createQueryStringForFacets(String field,
-                                               String value,
-                                               StringBuilder queryStringBuilder,
-                                               Monoflop ampersandPlaced) {
-        boolean fieldFound = false;
-        for (Facet f : getFacets()) {
-            if (Strings.areEqual(field, f.getName())) {
-                fieldFound = true;
-                if (Strings.isFilled(value)) {
-                    queryStringBuilder.append(ampersandPlaced.firstCall() ? "" : "&");
-                    queryStringBuilder.append(field);
-                    queryStringBuilder.append("=");
-                    queryStringBuilder.append(Strings.urlEncode(value));
-                }
-            } else if (Strings.isFilled(f.getValue())) {
-                queryStringBuilder.append(ampersandPlaced.firstCall() ? "" : "&");
-                queryStringBuilder.append(f.getName());
-                queryStringBuilder.append("=");
-                queryStringBuilder.append(Strings.urlEncode(f.getValue()));
-            }
-        }
-        return fieldFound;
-    }
 }
diff --git a/src/main/java/sirius/web/dispatch/AssetsDispatcher.java b/src/main/java/sirius/web/dispatch/AssetsDispatcher.java
@@ -172,9 +172,7 @@ private boolean trySASS(WebContext ctx, String uri, Response response) {
             try {
                 compileSASS(scssUri, file);
             } catch (Exception t) {
-                if (!file.delete()) {
-                    Templates.LOG.WARN("Cannot delete temporary file: %s", file.getAbsolutePath());
-                }
+                Files.delete(file);
                 ctx.respondWith().error(HttpResponseStatus.INTERNAL_SERVER_ERROR, Exceptions.handle(Templates.LOG, t));
                 return true;
             }

diff --git a/src/main/java/sirius/web/dispatch/HelpDispatcher.java b/src/main/java/sirius/web/dispatch/HelpDispatcher.java
@@ -86,10 +86,8 @@ public boolean dispatch(WebContext ctx) throws Exception {
         Tuple<String, String> langAndTopic = Strings.split(uri.substring(HELP_PREFIX_LENGTH), "/");
         boolean languageFound = setupLang(langAndTopic.getFirst());
 
-        if (!languageFound || Strings.isFilled(langAndTopic.getSecond())) {
-            if (serveTopic(ctx, uri)) {
-                return true;
-            }
+        if ((!languageFound || Strings.isFilled(langAndTopic.getSecond())) && serveTopic(ctx, uri)) {
+            return true;
         }
 
         if (!languageFound || Strings.areEqual(langAndTopic.getFirst(), NLS.getDefaultLanguage())) {

diff --git a/src/main/java/sirius/web/http/HttpPipeliningHandler.java b/src/main/java/sirius/web/http/HttpPipeliningHandler.java
@@ -55,11 +55,9 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
 
         // If a conflicting request was put aside in the bufferedRequests list, we can safely
         // ignore the empty  LastHttpContent for it - we will emulate this in <tt>write</tt>
-        if (msg instanceof LastHttpContent) {
-            if (((LastHttpContent) msg).content().readableBytes() == 0) {
-                ((LastHttpContent) msg).release();
-                return;
-            }
+        if (msg instanceof LastHttpContent && ((LastHttpContent) msg).content().readableBytes() == 0) {
+            ((LastHttpContent) msg).release();
+            return;
         }
 
         // If any other content is received (that would be another POST for example, we give up!) There

diff --git a/src/main/java/sirius/web/http/LowLevelHandler.java b/src/main/java/sirius/web/http/LowLevelHandler.java
@@ -35,15 +35,13 @@ public void connect(ChannelHandlerContext ctx,
             WebServer.connections.set(0);
         }
         IPRange.RangeSet filter = WebServer.getIPFilter();
-        if (!filter.isEmpty()) {
-            if (!filter.accepts(((InetSocketAddress) remoteAddress).getAddress())) {
-                if (WebServer.blocks.incrementAndGet() < 0) {
-                    WebServer.blocks.set(0);
-                }
-                ctx.channel().close();
-                future.setSuccess();
-                return;
+        if (!filter.isEmpty() && !filter.accepts(((InetSocketAddress) remoteAddress).getAddress())) {
+            if (WebServer.blocks.incrementAndGet() < 0) {
+                WebServer.blocks.set(0);
             }
+            ctx.channel().close();
+            future.setSuccess();
+            return;
         }
         super.connect(ctx, remoteAddress, localAddress, future);
     }