diff --git a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties index 3ff23eaf580f..e45f2c4b2b5d 100644 --- a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties +++ b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties @@ -245,6 +245,8 @@ use.obsolete.collection.type.problem.descriptor=Obsolete collection type # use.obsolete.collection.type.ignore.library.arguments.option=Ignore obsolete collection types where they are required use.system.out.err.display.name=Use of 'System.out' or 'System.err' use.system.out.err.problem.descriptor=Uses of #ref should probably be replaced with more robust logging #loc +use.system.out.err.problem.fix.err.option=Log method for 'System.err': +use.system.out.err.problem.fix.out.option=Log method for 'System.out': dumpstack.call.display.name=Call to 'Thread.dumpStack()' dumpstack.call.problem.descriptor=Call to Thread.#ref() should probably be replaced with more robust logging #loc printstacktrace.call.display.name=Call to 'printStackTrace()' @@ -1956,6 +1958,9 @@ invert.method.quickfix=Invert method invert.quickfix=Invert ''{0}'' throwable.printed.to.system.out.display.name='Throwable' printed to 'System.out' throwable.printed.to.system.out.problem.descriptor=''Throwable'' argument #ref to ''System.{0}.{1}()'' call +throwable.printed.to.system.out.problem.fix.level.option=Log method for fix: +convert.system.out.to.log.call.family.name=Convert ''System.out'' call to log call +convert.system.out.to.log.call.name=Convert ''System.out'' call to call of ''{0}'' suppress.for.tests.scope.quickfix=Suppress for 'Tests' scope implicit.default.charset.usage.display.name=Implicit platform default charset implicit.default.charset.usage.problem.descriptor=Call to #ref() uses the platform's default charset diff --git a/java/java-analysis-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java b/java/java-analysis-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java deleted file mode 100644 index 563f327c5037..000000000000 --- a/java/java-analysis-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright 2000-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.siyeh.ig.maturity; - -import com.intellij.psi.*; -import com.siyeh.InspectionGadgetsBundle; -import com.siyeh.ig.BaseInspection; -import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.TypeUtils; -import org.jetbrains.annotations.NonNls; -import org.jetbrains.annotations.NotNull; - -/** - * @author Bas Leijdekkers - */ -public final class ThrowablePrintedToSystemOutInspection extends BaseInspection { - - @NotNull - @Override - protected String buildErrorString(Object... infos) { - final String fieldName = (String)infos[0]; - final String methodName = (String)infos[1]; - return InspectionGadgetsBundle.message("throwable.printed.to.system.out.problem.descriptor", fieldName, methodName); - } - - @Override - public boolean isEnabledByDefault() { - return true; - } - - @Override - public BaseInspectionVisitor buildVisitor() { - return new ThrowablePrintedToSystemOutVisitor(); - } - - private static class ThrowablePrintedToSystemOutVisitor extends BaseInspectionVisitor { - - @Override - public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { - super.visitMethodCallExpression(expression); - final PsiReferenceExpression methodExpression = expression.getMethodExpression(); - @NonNls final String methodName = methodExpression.getReferenceName(); - if (!"print".equals(methodName) && !"println".equals(methodName)) { - return; - } - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 1) { - return; - } - final PsiExpression argument = arguments[0]; - if (!TypeUtils.expressionHasTypeOrSubtype(argument, CommonClassNames.JAVA_LANG_THROWABLE)) { - return; - } - final PsiMethod method = expression.resolveMethod(); - if (method == null) { - return; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (!(qualifier instanceof PsiReferenceExpression qualifierReference)) { - return; - } - final PsiElement target = qualifierReference.resolve(); - if (!(target instanceof PsiField field)) { - return; - } - @NonNls final String fieldName = field.getName(); - if (!"out".equals(fieldName) && !"err".equals(fieldName)) { - return; - } - final PsiClass aClass = field.getContainingClass(); - if (aClass == null || !"java.lang.System".equals(aClass.getQualifiedName())) { - return; - } - registerError(argument, fieldName, methodName); - } - } -} diff --git a/java/java-impl/src/META-INF/InspectionGadgets.xml b/java/java-impl/src/META-INF/InspectionGadgets.xml index 70e247d3df9b..1215ed1f4e72 100644 --- a/java/java-impl/src/META-INF/InspectionGadgets.xml +++ b/java/java-impl/src/META-INF/InspectionGadgets.xml @@ -1412,7 +1412,7 @@ StringConcatenationArgumentToLogCallFix.isAvailable(targetExpression) ? new StringConcatenationArgumentToLogCallFix() : null; @@ -168,7 +172,11 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn return new StringConcatenationArgumentToLogCallVisitor(); } - private static class StringConcatenationArgumentToLogCallFix extends PsiUpdateModCommandQuickFix { + public interface EvaluatedStringFix { + void fix(@NotNull Project project, @NotNull PsiMethodCallExpression logCall); + } + + private static class StringConcatenationArgumentToLogCallFix extends PsiUpdateModCommandQuickFix implements EvaluatedStringFix { StringConcatenationArgumentToLogCallFix() { } @@ -184,6 +192,11 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn if (!(grandParent instanceof PsiMethodCallExpression methodCallExpression)) { return; } + fix(project, methodCallExpression); + } + + @Override + public void fix(@NotNull Project project, @NotNull PsiMethodCallExpression methodCallExpression) { final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); final PsiExpression[] arguments = argumentList.getExpressions(); if (arguments.length == 0) { @@ -306,10 +319,16 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn newMethodCall.append('}'); } else { - for (PsiExpression newArgument : newArguments) { - newMethodCall.append(','); - if (newArgument != null) { - newMethodCall.append(newArgument.getText()); + if (newArguments.size() == 1 && newArguments.get(0) != null && + InheritanceUtil.isInheritor(newArguments.get(0).getType(), CommonClassNames.JAVA_LANG_THROWABLE)) { + newMethodCall.append(", String.valueOf(").append(newArguments.get(0).getText()).append(")"); + } + else { + for (PsiExpression newArgument : newArguments) { + newMethodCall.append(','); + if (newArgument != null) { + newMethodCall.append(newArgument.getText()); + } } } } @@ -351,7 +370,7 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn } } - private static abstract class FormatArgumentToLogCallFix extends PsiUpdateModCommandQuickFix { + private static abstract class FormatArgumentToLogCallFix extends PsiUpdateModCommandQuickFix implements EvaluatedStringFix { @NotNull private final Map myTextMapping; @@ -366,11 +385,7 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn } @Override - protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { - if (!(element.getParent() instanceof PsiReferenceExpression referenceExpression && - referenceExpression.getParent() instanceof PsiMethodCallExpression callExpression)) { - return; - } + public void fix(@NotNull Project project, @NotNull PsiMethodCallExpression callExpression) { PsiExpression[] expressions = callExpression.getArgumentList().getExpressions(); if (expressions.length != 1) { return; @@ -395,6 +410,15 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn tracker.replace(callExpression, builder.toString()); } + @Override + protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { + if (!(element.getParent() instanceof PsiReferenceExpression referenceExpression && + referenceExpression.getParent() instanceof PsiMethodCallExpression callExpression)) { + return; + } + fix(project, callExpression); + } + @NotNull private String createNewArgumentsFromCall(@NotNull PsiMethodCallExpression formatCallExpression, @NotNull CommentTracker tracker) { @@ -432,7 +456,7 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn @Nullable - static LocalQuickFix create(@NotNull PsiExpression expression) { + static PsiUpdateModCommandQuickFix create(@NotNull PsiExpression expression) { if (!(expression instanceof PsiMethodCallExpression callExpression)) { return null; } @@ -487,7 +511,7 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn } @Nullable - static LocalQuickFix create(@NotNull PsiExpression originalExpression) { + static PsiUpdateModCommandQuickFix create(@NotNull PsiExpression originalExpression) { if (!(originalExpression instanceof PsiMethodCallExpression callExpression)) { return null; } @@ -559,7 +583,7 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn } } - private enum ProblemType { + public enum ProblemType { CONCATENATION, STRING_FORMAT, MESSAGE_FORMAT } @@ -598,54 +622,73 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn if (arguments.length == 0) { return; } - if (arguments.length == 1 && arguments[0] instanceof PsiMethodCallExpression callExpression) { - FormatDecode.FormatArgument formatArgument = - FormatDecode.FormatArgument.extract(callExpression, List.of("format"), List.of("String"), true); - if (formatArgument != null) { - registerMethodCallError(expression, ProblemType.STRING_FORMAT, expression, callExpression); - return; - } - if (MESSAGE_FORMAT_FORMAT.test(callExpression)) { - registerMethodCallError(expression, ProblemType.MESSAGE_FORMAT, expression, callExpression); - return; - } - } - PsiExpression argument = arguments[0]; - if (!ExpressionUtils.hasStringType(argument)) { - if (arguments.length < 2) { - return; - } - argument = arguments[1]; - if (!ExpressionUtils.hasStringType(argument)) { - return; - } - } - if (!containsNonConstantConcatenation(argument)) { - return; - } - registerMethodCallError(expression, ProblemType.CONCATENATION, expression, argument); - } + LogConcatenationContext result = getLogConcatenationContext(arguments); + if (result == null) return; - private static boolean containsNonConstantConcatenation(@Nullable PsiExpression expression) { - if (expression instanceof PsiParenthesizedExpression parenthesizedExpression) { - return containsNonConstantConcatenation(parenthesizedExpression.getExpression()); - } - else if (expression instanceof PsiPolyadicExpression polyadicExpression) { - if (!ExpressionUtils.hasStringType(polyadicExpression)) { - return false; - } - if (!JavaTokenType.PLUS.equals(polyadicExpression.getOperationTokenType())) { - return false; - } - final PsiExpression[] operands = polyadicExpression.getOperands(); - for (PsiExpression operand : operands) { - if (!ExpressionUtils.isEvaluatedAtCompileTime(operand)) { - return true; - } - } - } - return false; + registerMethodCallError(expression, result.problemType(), expression, result.argument()); } } + + + public record LogConcatenationContext(@NotNull PsiExpression argument, @NotNull ProblemType problemType) { + } + + @Nullable + public static LogConcatenationContext getLogConcatenationContext(PsiExpression @NotNull [] arguments) { + PsiExpression argument = arguments[0]; + + ProblemType problemType = null; + + if (arguments.length == 1 && argument instanceof PsiMethodCallExpression callExpression) { + FormatDecode.FormatArgument formatArgument = + FormatDecode.FormatArgument.extract(callExpression, List.of("format"), List.of("String"), true); + if (formatArgument != null) { + problemType = ProblemType.STRING_FORMAT; + } + else if (MESSAGE_FORMAT_FORMAT.test(callExpression)) { + problemType = ProblemType.MESSAGE_FORMAT; + } + } + + if (problemType == null) { + if (!ExpressionUtils.hasStringType(argument)) { + if (arguments.length < 2) { + return null; + } + argument = arguments[1]; + } + if (!ExpressionUtils.hasStringType(argument)) { + return null; + } + + if (!containsNonConstantConcatenation(argument)) { + return null; + } + problemType = ProblemType.CONCATENATION; + } + + return new LogConcatenationContext(argument, problemType); + } + + private static boolean containsNonConstantConcatenation(@Nullable PsiExpression expression) { + if (expression instanceof PsiParenthesizedExpression parenthesizedExpression) { + return containsNonConstantConcatenation(parenthesizedExpression.getExpression()); + } + else if (expression instanceof PsiPolyadicExpression polyadicExpression) { + if (!ExpressionUtils.hasStringType(polyadicExpression)) { + return false; + } + if (!JavaTokenType.PLUS.equals(polyadicExpression.getOperationTokenType())) { + return false; + } + final PsiExpression[] operands = polyadicExpression.getOperands(); + for (PsiExpression operand : operands) { + if (!ExpressionUtils.isEvaluatedAtCompileTime(operand)) { + return true; + } + } + } + return false; + } } \ No newline at end of file diff --git a/java/java-impl/src/com/siyeh/ig/maturity/ConvertSystemOutToLogCallFix.java b/java/java-impl/src/com/siyeh/ig/maturity/ConvertSystemOutToLogCallFix.java new file mode 100644 index 000000000000..a50b139310bc --- /dev/null +++ b/java/java-impl/src/com/siyeh/ig/maturity/ConvertSystemOutToLogCallFix.java @@ -0,0 +1,301 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.siyeh.ig.maturity; + +import com.intellij.java.JavaBundle; +import com.intellij.lang.logging.JvmLogger; +import com.intellij.modcommand.*; +import com.intellij.openapi.module.Module; +import com.intellij.openapi.module.ModuleUtilCore; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.NlsSafe; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.InheritanceUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.ui.logging.JvmLoggingSettingsStorage; +import com.intellij.util.containers.ContainerUtil; +import com.siyeh.HardcodedMethodConstants; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.callMatcher.CallMatcher; +import com.siyeh.ig.logging.StringConcatenationArgumentToLogCallInspection; +import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.VariableNameGenerator; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.stream.Collectors; + +import static com.intellij.lang.logging.UnspecifiedLogger.UNSPECIFIED_LOGGER_ID; +import static com.intellij.psi.CommonClassNames.JAVA_IO_PRINT_STREAM; +import static com.intellij.psi.CommonClassNames.JAVA_LANG_SYSTEM; + +public class ConvertSystemOutToLogCallFix extends PsiBasedModCommandAction { + + private static final CallMatcher myCallMatcher = CallMatcher.instanceCall(JAVA_IO_PRINT_STREAM, "println", "print"); + private static final Set SUPPORTED_LOGGING_SYSTEMS = Set.of("Log4j2", "Slf4j", "Lombok Slf4j", "Lombok Log4j2"); + + @NotNull + private final List myLoggers; + + @NotNull + private final String myMethodName; + + @Nullable + private final String myLogName; + + private ConvertSystemOutToLogCallFix(@NotNull List loggers, + @Nullable String logName, + @NotNull String method, + @NotNull PsiMethodCallExpression logCall) { + super(logCall); + myLoggers = loggers; + myMethodName = method; + myLogName = logName; + } + + @Override + public @NotNull String getFamilyName() { + if (myLoggers.size() != 1) { + return InspectionGadgetsBundle.message("convert.system.out.to.log.call.family.name"); + } + return InspectionGadgetsBundle.message("convert.system.out.to.log.call.name", myLoggers.get(0).toString()); + } + + @Override + protected @NotNull ModCommand perform(@NotNull ActionContext context, @NotNull PsiMethodCallExpression originalCall) { + if (myLoggers.isEmpty()) { + return ModCommand.nop(); + } + + if (myLogName != null) { + return ModCommand.psiUpdate(originalCall, logCall -> replaceWithLogCall(logCall, myLogName, myMethodName)); + } + + if (myLoggers.size() > 1) { + return ModCommand.chooseAction(JavaBundle.message("dialog.title.choose.logger"), + ContainerUtil.map(myLoggers, + log -> new ConvertSystemOutToLogCallFix(List.of(log), null, myMethodName, + originalCall))); + } + + return ModCommand.psiUpdate(originalCall, callExpression -> { + JvmLogger logger = myLoggers.get(0); + + PsiClass upperClass = getUpperClass(callExpression); //not always a good idea for all cases, but asking is too annoying + + if (upperClass == null) { + return; + } + + String logFieldName = logger.getLogFieldName(upperClass); + if (logFieldName == null) { + return; + } + + Project project = context.project(); + PsiElement loggerElement = logger.createLogger(project, upperClass); + if (loggerElement == null) { + return; + } + logger.insertLoggerAtClass(project, upperClass, loggerElement); + + replaceWithLogCall(callExpression, logFieldName, myMethodName); + }); + } + + private static void replaceWithLogCall(@NotNull PsiMethodCallExpression callExpression, + @NotNull String logName, + @NotNull String logMethodName) { + PsiExpression[] expressions = callExpression.getArgumentList().getExpressions(); + if(expressions.length != 1) { + return; + } + + PsiExpression expression = expressions[0]; + Project project = callExpression.getProject(); + + String arguments = getArguments(expression); + + String text = logName + "." + logMethodName + "(" + arguments + ")"; + + CommentTracker tracker = new CommentTracker(); + PsiElement newElement = tracker.replace(callExpression, text); + + if (!(newElement instanceof PsiMethodCallExpression newCall)) { + return; + } + StringConcatenationArgumentToLogCallInspection.LogConcatenationContext context = + StringConcatenationArgumentToLogCallInspection.getLogConcatenationContext(newCall.getArgumentList().getExpressions()); + + if (context == null) { + return; + } + + PsiUpdateModCommandQuickFix fix = StringConcatenationArgumentToLogCallInspection.getQuickFix(context.problemType(), context.argument()); + if (fix instanceof StringConcatenationArgumentToLogCallInspection.EvaluatedStringFix evaluatedStringFix) { + evaluatedStringFix.fix(project, newCall); + } + } + + @NotNull + private static String getArguments(@NotNull PsiExpression expression) { + PsiType expressionType = expression.getType(); + + if (expressionType==null ||expressionType.equalsToText(CommonClassNames.JAVA_LANG_STRING)) { + return expression.getText(); + } + + if (InheritanceUtil.isInheritor(expressionType, CommonClassNames.JAVA_LANG_EXCEPTION)) { + return "\"e: \" , " + expression.getText(); + } + + return "String.valueOf(" + expression.getText() + ")"; + } + + @Nullable + public static ModCommandAction createFix(@NotNull PsiMethodCallExpression callExpression, + @NotNull String method) { + if (!myCallMatcher.matches(callExpression)) { + return null; + } + + PsiReferenceExpression expressionPrint = callExpression.getMethodExpression(); + if (!(expressionPrint.getQualifierExpression() instanceof PsiReferenceExpression expression)) { + return null; + } + String name = expression.getReferenceName(); + if (!HardcodedMethodConstants.OUT.equals(name) && + !HardcodedMethodConstants.ERR.equals(name)) { + return null; + } + + PsiElement referent = expression.resolve(); + if (!(referent instanceof PsiField systemField)) { + return null; + } + + final PsiClass containingClass = systemField.getContainingClass(); + if (containingClass == null) { + return null; + } + final String className = containingClass.getQualifiedName(); + if (!JAVA_LANG_SYSTEM.equals(className)) { + return null; + } + + if (callExpression.getArgumentList().getExpressions().length != 1) { + return null; + } + + Module module = ModuleUtilCore.findModuleForFile(callExpression.getContainingFile()); + if (module == null) { + return null; + } + + List loggers = JvmLogger.Companion.findSuitableLoggers(module, true); + + if (loggers.isEmpty()) { + return null; + } + + PsiClass psiClass = getUpperClass(callExpression); + if (psiClass == null) { + return null; + } + + Project project = callExpression.getProject(); + GlobalSearchScope globalSearchScope = GlobalSearchScope.everythingScope(project); + List loggersWithTargetMethod = new ArrayList<>(); + for (JvmLogger logger : loggers) { + PsiClass loggerClass = JavaPsiFacade.getInstance(project).findClass(logger.getLoggerTypeName(), globalSearchScope); + if (loggerClass != null && loggerClass.findMethodsByName(method, true).length > 0 && + SUPPORTED_LOGGING_SYSTEMS.contains(logger.getId())) { + loggersWithTargetMethod.add(logger); + } + } + + if (loggersWithTargetMethod.isEmpty()) { + return null; + } + + List existedLoggers = new ArrayList<>(); + PsiField[] fields = psiClass.getFields(); + Set allSupportedLoggers = loggersWithTargetMethod.stream().map(log -> log.getLoggerTypeName()).collect(Collectors.toSet()); + for (PsiField field : fields) { + if (allSupportedLoggers.contains(field.getType().getCanonicalText())) { + existedLoggers.add(field); + } + } + + if (!existedLoggers.isEmpty()) { + String possibleLogName = existedLoggers.get(0).getName(); + PsiResolveHelper helper = JavaPsiFacade.getInstance(project).getResolveHelper(); + PsiVariable variable = helper.resolveAccessibleReferencedVariable(possibleLogName, callExpression); + if (variable instanceof PsiField field && + allSupportedLoggers.contains(field.getType().getCanonicalText())) { + return new ConvertSystemOutToLogCallFix(loggersWithTargetMethod, possibleLogName, method, callExpression); + } + return null; + } + + List availableLoggers = new ArrayList<>(); + for (JvmLogger logger: loggersWithTargetMethod) { + String logFieldName = logger.getLogFieldName(psiClass); + if (logFieldName == null) { + continue; + } + String proposedName = new VariableNameGenerator(callExpression, VariableKind.LOCAL_VARIABLE).byName(logFieldName) + .generate(true); + if (!proposedName.equals(logFieldName)) { + continue; + } + + if (logger.isPossibleToPlaceLoggerAtClass(psiClass)) { + availableLoggers.add(logger); + } + } + + if (availableLoggers.isEmpty()) { + return null; + } + + JvmLoggingSettingsStorage loggingSettingsStorage = project.getService(JvmLoggingSettingsStorage.class); + JvmLoggingSettingsStorage.State state = loggingSettingsStorage.getState(); + String id = state.getLoggerId(); + + if (UNSPECIFIED_LOGGER_ID.equals(id)) { + return new ConvertSystemOutToLogCallFix(availableLoggers, null, method, callExpression); + } + + Optional chosenLogger = availableLoggers.stream().filter(log -> log.getId().equals(id)).findAny(); + if (chosenLogger.isEmpty()) { + return null; + } + + return new ConvertSystemOutToLogCallFix(chosenLogger.stream().toList(), null, method, callExpression); + } + + private static @Nullable PsiClass getUpperClass(@NotNull PsiElement source) { + PsiElement target = source; + PsiElement next = source; + while (next != null) { + target = next; + next = PsiTreeUtil.findFirstParent(target, true, + parent -> parent instanceof PsiClass && + !(parent instanceof PsiImplicitClass) && + !(parent instanceof PsiAnonymousClass)); + } + return target instanceof PsiClass ? (PsiClass)target : null; + } + + public enum PopularLogLevel { + TRACE, DEBUG, INFO, WARN, ERROR; + + @NlsSafe + public String toMethodName() { + return name().toLowerCase(Locale.ROOT); + } + } +} diff --git a/java/java-impl/src/com/siyeh/ig/maturity/SystemOutErrInspection.java b/java/java-impl/src/com/siyeh/ig/maturity/SystemOutErrInspection.java index dfce676dde8e..84cb3c816d8d 100644 --- a/java/java-impl/src/com/siyeh/ig/maturity/SystemOutErrInspection.java +++ b/java/java-impl/src/com/siyeh/ig/maturity/SystemOutErrInspection.java @@ -2,28 +2,83 @@ package com.siyeh.ig.maturity; import com.intellij.codeInspection.LocalQuickFix; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiField; -import com.intellij.psi.PsiReferenceExpression; +import com.intellij.codeInspection.options.OptPane; +import com.intellij.modcommand.ModCommandAction; +import com.intellij.modcommand.ModCommandService; +import com.intellij.profile.codeInspection.InspectionProjectProfileManager; +import com.intellij.psi.*; import com.siyeh.HardcodedMethodConstants; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.fixes.SuppressForTestsScopeFix; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_SYSTEM; /** * @author Bas Leijdekkers */ public final class SystemOutErrInspection extends BaseInspection { - @Nullable + public ConvertSystemOutToLogCallFix.PopularLogLevel myErrLogLevel = ConvertSystemOutToLogCallFix.PopularLogLevel.ERROR; + public ConvertSystemOutToLogCallFix.PopularLogLevel myOutLogLevel = ConvertSystemOutToLogCallFix.PopularLogLevel.INFO; + + @Override - protected LocalQuickFix buildFix(Object... infos) { + public @NotNull OptPane getOptionsPane() { + return OptPane.pane( + OptPane.dropdown( + "myErrLogLevel", + InspectionGadgetsBundle.message("use.system.out.err.problem.fix.err.option"), + ConvertSystemOutToLogCallFix.PopularLogLevel.class, + level -> level.toMethodName()), + OptPane.dropdown( + "myOutLogLevel", + InspectionGadgetsBundle.message("use.system.out.err.problem.fix.out.option"), + ConvertSystemOutToLogCallFix.PopularLogLevel.class, + level -> level.toMethodName()) + ); + } + + @Override + protected LocalQuickFix @NotNull [] buildFixes(Object... infos) { + List fixes = new ArrayList<>(); + final PsiElement context = (PsiElement)infos[0]; - return SuppressForTestsScopeFix.build(this, context); + + SuppressForTestsScopeFix testsScopeFix = SuppressForTestsScopeFix.build(this, context); + if(testsScopeFix != null) { + fixes.add(testsScopeFix); + } + + if (context instanceof PsiReferenceExpression referenceExpression && + referenceExpression.getParent() instanceof PsiReferenceExpression probablyReferenceToCall && + probablyReferenceToCall.getParent() instanceof PsiMethodCallExpression callExpression) { + + String name = referenceExpression.getReferenceName(); + String methodName = null; + if (HardcodedMethodConstants.OUT.equals(name)) { + methodName = myOutLogLevel.name().toLowerCase(Locale.ROOT); + } + else if (HardcodedMethodConstants.ERR.equals(name)) { + methodName = myErrLogLevel.name().toLowerCase(Locale.ROOT); + } + + if (methodName != null) { + ModCommandAction fix = ConvertSystemOutToLogCallFix.createFix(callExpression, methodName); + if (fix != null) { + LocalQuickFix localQuickFix = ModCommandService.getInstance().wrapToQuickFix(fix); + fixes.add(localQuickFix); + } + } + } + + return fixes.toArray(LocalQuickFix.EMPTY_ARRAY); } @Override @@ -44,11 +99,28 @@ public final class SystemOutErrInspection extends BaseInspection { return new SystemOutErrVisitor(); } - private static class SystemOutErrVisitor extends BaseInspectionVisitor { + private class SystemOutErrVisitor extends BaseInspectionVisitor { + + @Override + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + PsiReferenceExpression methodExpression = expression.getMethodExpression(); + if (methodExpression.getQualifierExpression() instanceof PsiReferenceExpression referenceExpression) { + inspectReferenceIfItIsSystemOutErr(referenceExpression); + } + } @Override public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) { super.visitReferenceExpression(expression); + if (expression.getParent() instanceof PsiReferenceExpression parentReferenceExpression && + parentReferenceExpression.getParent() instanceof PsiMethodCallExpression) { + return; + } + inspectReferenceIfItIsSystemOutErr(expression); + } + + private void inspectReferenceIfItIsSystemOutErr(@NotNull PsiReferenceExpression expression) { final String name = expression.getReferenceName(); if (!HardcodedMethodConstants.OUT.equals(name) && !HardcodedMethodConstants.ERR.equals(name)) { @@ -63,9 +135,23 @@ public final class SystemOutErrInspection extends BaseInspection { return; } final String className = containingClass.getQualifiedName(); - if (!"java.lang.System".equals(className)) { + if (!JAVA_LANG_SYSTEM.equals(className)) { return; } + + if (expression.getParent() instanceof PsiReferenceExpression probablyReferenceToCall && + probablyReferenceToCall.getParent() instanceof PsiMethodCallExpression callExpression) { + + if (ThrowablePrintedToSystemOutInspection.getExceptionIsPrintedToSystemOutResult(callExpression) != null) { + return; + } + + boolean informationLevel = InspectionProjectProfileManager.isInformationLevel(getShortName(), expression); + if (informationLevel) { + registerError(callExpression, expression); + } + } + registerError(expression, expression); } } diff --git a/java/java-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java b/java/java-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java new file mode 100644 index 000000000000..115457983d50 --- /dev/null +++ b/java/java-impl/src/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspection.java @@ -0,0 +1,131 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.siyeh.ig.maturity; + +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.options.OptPane; +import com.intellij.modcommand.ModCommandAction; +import com.intellij.modcommand.ModCommandService; +import com.intellij.psi.*; +import com.siyeh.HardcodedMethodConstants; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; +import com.siyeh.ig.BaseInspectionVisitor; +import com.siyeh.ig.psiutils.TypeUtils; +import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Locale; + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_SYSTEM; + +/** + * @author Bas Leijdekkers + */ +public final class ThrowablePrintedToSystemOutInspection extends BaseInspection { + + public ConvertSystemOutToLogCallFix.PopularLogLevel myLogLevel = ConvertSystemOutToLogCallFix.PopularLogLevel.ERROR; + + @Override + public @NotNull OptPane getOptionsPane() { + return OptPane.pane( + OptPane.dropdown( + "myLogLevel", + InspectionGadgetsBundle.message("throwable.printed.to.system.out.problem.fix.level.option"), + ConvertSystemOutToLogCallFix.PopularLogLevel.class, + level -> level.toMethodName()) + ); + } + + @NotNull + @Override + protected String buildErrorString(Object... infos) { + final String fieldName = (String)infos[0]; + final String methodName = (String)infos[1]; + return InspectionGadgetsBundle.message("throwable.printed.to.system.out.problem.descriptor", fieldName, methodName); + } + + @Override + public boolean isEnabledByDefault() { + return true; + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new ThrowablePrintedToSystemOutVisitor(); + } + + @Override + protected @Nullable LocalQuickFix buildFix(Object... infos) { + if (infos.length != 3) { + return null; + } + if (!(infos[2] instanceof PsiExpression expression && + expression.getParent() instanceof PsiExpressionList expressionList && + expressionList.getParent() instanceof PsiMethodCallExpression call)) { + return null; + } + + ModCommandAction fix = ConvertSystemOutToLogCallFix.createFix(call, myLogLevel.name().toLowerCase(Locale.ROOT)); + if (fix != null) { + return ModCommandService.getInstance().wrapToQuickFix(fix); + } + return null; + } + + private static class ThrowablePrintedToSystemOutVisitor extends BaseInspectionVisitor { + + @Override + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + ExceptionIsPrintedToSystemOutResult result = getExceptionIsPrintedToSystemOutResult(expression); + if (result == null) return; + registerError(result.argument(), result.fieldName(), result.methodName(), result.argument()); + } + } + + public static @Nullable ExceptionIsPrintedToSystemOutResult getExceptionIsPrintedToSystemOutResult(@NotNull PsiMethodCallExpression expression) { + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + @NonNls final String methodName = methodExpression.getReferenceName(); + if (!"print".equals(methodName) && !"println".equals(methodName)) { + return null; + } + final PsiExpressionList argumentList = expression.getArgumentList(); + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length != 1) { + return null; + } + final PsiExpression argument = arguments[0]; + + if (!TypeUtils.expressionHasTypeOrSubtype(argument, CommonClassNames.JAVA_LANG_THROWABLE)) { + return null; + } + final PsiMethod method = expression.resolveMethod(); + if (method == null) { + return null; + } + final PsiExpression qualifier = methodExpression.getQualifierExpression(); + if (!(qualifier instanceof PsiReferenceExpression qualifierReference)) { + return null; + } + final PsiElement target = qualifierReference.resolve(); + if (!(target instanceof PsiField field)) { + return null; + } + @NonNls final String fieldName = field.getName(); + if (!HardcodedMethodConstants.OUT.equals(fieldName) && !HardcodedMethodConstants.ERR.equals(fieldName)) { + return null; + } + final PsiClass aClass = field.getContainingClass(); + if (aClass == null || !JAVA_LANG_SYSTEM.equals(aClass.getQualifiedName())) { + return null; + } + + return new ExceptionIsPrintedToSystemOutResult(methodName, argument, fieldName); + } + + public record ExceptionIsPrintedToSystemOutResult(@NotNull String methodName, + @NotNull PsiExpression argument, + @NotNull String fieldName) { + } +} diff --git a/java/java-impl/src/inspectionDescriptions/SystemOutErr.html b/java/java-impl/src/inspectionDescriptions/SystemOutErr.html index b880766b69b0..047c9746df3e 100644 --- a/java/java-impl/src/inspectionDescriptions/SystemOutErr.html +++ b/java/java-impl/src/inspectionDescriptions/SystemOutErr.html @@ -3,6 +3,28 @@ Reports usages of System.out or System.err.

Such statements are often used for temporary debugging and should be either removed from the production code, or replaced by a more robust logging facility.

+ +

The provided quick-fix supports SLF4J and Log4j 2. + It replaces System.out and System.err calls with log calls

+ +

Example:

+

+  public static void test(Object o) {
+    System.out.println("Test: "+ o);
+  }
+
+

After the quick-fix is applied:

+

+  private static final Logger log = LoggerFactory.getLogger(Test.class);
+
+  public static void test(Object o) {
+      log.info("Test: {}", o);;
+  }
+
+

+ Use the Log method for 'System.err' option to specify a method which it is used to log a message for 'System.err' calls. + Use the Log method for 'System.out' option to specify a method which it is used to log a message for 'System.out' calls. +

\ No newline at end of file diff --git a/java/java-impl/src/inspectionDescriptions/ThrowablePrintedToSystemOut.html b/java/java-impl/src/inspectionDescriptions/ThrowablePrintedToSystemOut.html index 43a7b372a5a0..9395ff4e9a95 100644 --- a/java/java-impl/src/inspectionDescriptions/ThrowablePrintedToSystemOut.html +++ b/java/java-impl/src/inspectionDescriptions/ThrowablePrintedToSystemOut.html @@ -5,8 +5,9 @@ Reports calls to System.out.println() with an exception as an argum It is recommended that you use logger instead.

Calls to System.out.print(), System.err.println(), and System.err.print() with an exception argument are also reported. It is better to use a logger to log exceptions instead.

- -

For example, instead of:

+

The provided quick-fix supports SLF4J and Log4j 2. +It replaces System.out.println() call with log calls

+

Example:


   try {
       foo();
@@ -14,14 +15,17 @@ reported. It is better to use a logger to log exceptions instead.

       System.out.println(e);
   }
-

use the following code:

+

After the quick-fix is applied:


   try {
       foo();
   } catch (Exception e) {
-      logger.warn(e); // logger call may be different
+    log.error("e: ", e);
   }
- + +

+ Use the Log method for fix option to specify a method which it is used to log a message. +

\ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/maturity/SystemOutErrInspectionTest.java b/java/java-tests/testSrc/com/siyeh/ig/maturity/SystemOutErrInspectionTest.java index a07750571046..04a090206cd9 100644 --- a/java/java-tests/testSrc/com/siyeh/ig/maturity/SystemOutErrInspectionTest.java +++ b/java/java-tests/testSrc/com/siyeh/ig/maturity/SystemOutErrInspectionTest.java @@ -36,6 +36,190 @@ public class SystemOutErrInspectionTest extends LightJavaInspectionTestCase { "}"); } + private void addSlf4j() { + addEnvironmentClass(""" + package org.slf4j; + @SuppressWarnings("ALL") public class LoggerFactory { + public static Logger getLogger(Class clazz) { return null; } + } + public interface Logger { + void error(String format, Object... arguments); + void info(String format, Object... arguments); + } + """); + } + + private void addMessageFormat() { + addEnvironmentClass(""" + package java.text; + public final class MessageFormat{ + public static String format(String format, Object... arguments) {return format;} + } + """); + } + + public void testSimpleLogFix() { + addSlf4j(); + + doTest( + """ + class Test{ + void foo(Object o) { + /*Uses of 'System.out' should probably be replaced with more robust logging*/System.out/**/.println("Something " + o); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Object o) { + log.info("Something {}", o); + } + } + """); + } + + public void testSimpleExistedLogFix() { + addSlf4j(); + + doTest( + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Exception o) { + /*Uses of 'System.out' should probably be replaced with more robust logging*/System.out/**/.println("Something " + o); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Exception o) { + log.info("Something {}", String.valueOf(o)); + } + } + """); + } + + public void testSimpleExistedLogWithRecordFix() { + addSlf4j(); + + doTest( + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + record R(){} + + void foo(R r) { + /*Uses of 'System.out' should probably be replaced with more robust logging*/System.out/**/.println(r); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + record R(){} + + void foo(R r) { + log.info(String.valueOf(r)); + } + } + """); + } + + public void testSimpleExistedLogWithStringFormatFix() { + addSlf4j(); + + doTest( + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Object r) { + /*Uses of 'System.out' should probably be replaced with more robust logging*/System.out/**/.println(String.format("test %s test", r)); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Object r) { + log.info("test {} test", r); + } + } + """); + } + + public void testSimpleExistedLogWithMessageFormatFix() { + addSlf4j(); + addMessageFormat(); + + doTest( + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + import java.text.MessageFormat; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Object r) { + /*Uses of 'System.out' should probably be replaced with more robust logging*/System.out/**/.println(MessageFormat.format("test {0} test", r)); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + import java.text.MessageFormat; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo(Object r) { + log.info("test {} test", r); + } + } + """); + } + @Override protected InspectionProfileEntry getInspection() { return new SystemOutErrInspection(); diff --git a/java/java-tests/testSrc/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspectionTest.java b/java/java-tests/testSrc/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspectionTest.java index ef4273c6214f..27eb6961e2f2 100644 --- a/java/java-tests/testSrc/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspectionTest.java +++ b/java/java-tests/testSrc/com/siyeh/ig/maturity/ThrowablePrintedToSystemOutInspectionTest.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2013 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.siyeh.ig.maturity; import com.intellij.codeInspection.InspectionProfileEntry; @@ -36,6 +22,83 @@ public class ThrowablePrintedToSystemOutInspectionTest extends LightJavaInspecti doStatementTest("System.out.println(/*'Throwable' argument 'new RuntimeException()' to 'System.out.println()' call*/new RuntimeException()/**/);"); } + @SuppressWarnings("ThrowableNotThrown") + public void testSimpleLogFix() { + addSlf4j(); + + doTest( + """ + class Test{ + void foo() { + final RuntimeException x = new RuntimeException(); + System.out.println(/*'Throwable' argument 'x' to 'System.out.println()' call*/x/**/); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger log = LoggerFactory.getLogger(Test.class); + + void foo() { + final RuntimeException x = new RuntimeException(); + log.error("e: ", x); + } + } + """); + } + + private void addSlf4j() { + addEnvironmentClass(""" + package org.slf4j; + @SuppressWarnings("ALL") public class LoggerFactory { + public static Logger getLogger(Class clazz) { return null; } + } + public interface Logger { + void error(String format, Object... arguments); + } + """); + } + + @SuppressWarnings("ThrowableNotThrown") + public void testSimpleExistedLogFix() { + addSlf4j(); + + doTest( + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger logger = LoggerFactory.getLogger(Test.class); + + void foo() { + final RuntimeException x = new RuntimeException(); + System.out.println(/*'Throwable' argument 'x' to 'System.out.println()' call*/x/**/); + } + } + """); + + checkQuickFix("Convert 'System.out' call to call of 'Slf4j'", + """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + + class Test{ + private static final Logger logger = LoggerFactory.getLogger(Test.class); + + void foo() { + final RuntimeException x = new RuntimeException(); + logger.error("e: ", x); + } + } + """); + } + @Override protected InspectionProfileEntry getInspection() { return new ThrowablePrintedToSystemOutInspection();