-
Notifications
You must be signed in to change notification settings - Fork 34
LoggerLevelArgumentToMethod for JBoss Logging #243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
fb0528b
0ea11c2
b74f286
704b3f2
a6e6b0e
72edcc6
45155b0
d534c7d
6d276d2
3f8e2a0
d7f3289
6b28f2b
6f2f0bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /* | ||
| * Copyright 2025 the original author or authors. | ||
| * <p> | ||
| * Licensed under the Moderne Source Available License (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * <p> | ||
| * https://docs.moderne.io/licensing/moderne-source-available-license | ||
| * <p> | ||
| * 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 org.openrewrite.java.logging.jboss; | ||
|
|
||
| import org.jspecify.annotations.Nullable; | ||
| import org.openrewrite.ExecutionContext; | ||
| import org.openrewrite.Preconditions; | ||
| import org.openrewrite.Recipe; | ||
| import org.openrewrite.TreeVisitor; | ||
| import org.openrewrite.internal.ListUtils; | ||
| import org.openrewrite.java.JavaIsoVisitor; | ||
| import org.openrewrite.java.MethodMatcher; | ||
| import org.openrewrite.java.search.UsesMethod; | ||
| import org.openrewrite.java.search.UsesType; | ||
| import org.openrewrite.java.tree.Expression; | ||
| import org.openrewrite.java.tree.J; | ||
| import org.openrewrite.java.tree.JavaType; | ||
| import org.openrewrite.java.tree.TypeUtils; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public class LoggerLevelArgumentToMethod extends Recipe { | ||
| private static final MethodMatcher LOG_MATCHER = new MethodMatcher("org.jboss.logging.Logger log(*,*,..)", true); | ||
| private static final MethodMatcher LOGF_MATCHER = new MethodMatcher("org.jboss.logging.Logger logf(*,*,..)", true); | ||
| private static final MethodMatcher LOGV_MATCHER = new MethodMatcher("org.jboss.logging.Logger logv(*,*,..)", true); | ||
|
timtebeek marked this conversation as resolved.
|
||
|
|
||
| @Override | ||
| public String getDisplayName() { | ||
| return "Replace JBoss Logging Level arguments with the corresponding eponymous level method calls"; | ||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return "Replace calls to `Logger.log(Level, ...)` with the corresponding eponymous level method calls. For example `Logger.log(Level.INFO, ...)` to `Logger.info(...)`."; | ||
| } | ||
|
|
||
| @Override | ||
| public TreeVisitor<?, ExecutionContext> getVisitor() { | ||
| TreeVisitor<?, ExecutionContext> preconditions = Preconditions.and( | ||
| new UsesType<>("org.jboss.logging.Logger", true), | ||
| new UsesType<>("org.jboss.logging.Logger.Level", true), | ||
| Preconditions.or( | ||
| new UsesMethod<>(LOG_MATCHER), | ||
| new UsesMethod<>(LOGF_MATCHER), | ||
| new UsesMethod<>(LOGV_MATCHER) | ||
| ) | ||
| ); | ||
| return Preconditions.check(preconditions, new JavaIsoVisitor<ExecutionContext>() { | ||
| @Override | ||
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) { | ||
| J.MethodInvocation m = super.visitMethodInvocation(mi, ctx); | ||
| if (!(LOG_MATCHER.matches(m) || LOGF_MATCHER.matches(m) || LOGV_MATCHER.matches(m))) { | ||
| return m; | ||
| } | ||
| List<Expression> args = m.getArguments(); | ||
| Expression firstArgument = args.get(0); | ||
| Expression secondArgument = args.get(1); | ||
|
|
||
| String logLevelName; | ||
| List<Expression> updatedArguments; | ||
| if (TypeUtils.isAssignableTo("org.jboss.logging.Logger.Level", firstArgument.getType())) { | ||
| String suffix = ""; | ||
| if (LOGF_MATCHER.matches(m)) { | ||
| suffix = "f"; | ||
| } else if (LOGV_MATCHER.matches(m)) { | ||
| suffix = "v"; | ||
| } | ||
| logLevelName = extractLogLevelName(firstArgument); | ||
| if (logLevelName != null) { | ||
| logLevelName += suffix; | ||
| } | ||
| updatedArguments = ListUtils.concat( | ||
| (Expression) secondArgument.withPrefix(firstArgument.getPrefix()), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering what the best thing to do is here regarding the Space. Right now replacing the Space is not good enough, because what happens now would be something like this: - logger.log( /* comment 1 */ Logger.Level.TRACE, /* comment 2 */ msg, t);
+ logger.trace( /* comment 1 */ msg, t);This does not make any sense to me. We can just remove the first argument, like you do for the other case: pdatedArguments = ListUtils.filter(args, it -> it != firstArgument);That would remove the item including the whitespace and comments: - logger.log( /* comment 1 */ Logger.Level.TRACE, /* comment 2 */ msg, t);
+ logger.trace( /* comment 2 */ msg, t);Another option would be to merge the comments and keep only the whitespace of the second argument. Maybe that's the best thing you can do. Then it would be something like: - logger.log( /* comment 1 */ Logger.Level.TRACE, /* comment 2 */ msg, t);
+ logger.trace(/* comment 1 */ /* comment 2 */ msg, t);Personally, I would just remove the first argument. If there is a comment, big chances are that this comment is no longer relevant. The same applies to the other case!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Applied in 6f2f0bf |
||
| args.subList(2, args.size())); | ||
| } else if (TypeUtils.isAssignableTo("java.lang.String", firstArgument.getType()) && | ||
| TypeUtils.isAssignableTo("org.jboss.logging.Logger.Level", secondArgument.getType()) && | ||
| LOG_MATCHER.matches(m)) { // `logf(String, ..)` and `logv(String, ..)` don't have a logger.level() equivalent | ||
| logLevelName = extractLogLevelName(secondArgument); | ||
| updatedArguments = ListUtils.filter(args, it -> it != secondArgument); | ||
| } else { | ||
| return m; | ||
| } | ||
|
|
||
| // If we can't extract a log level name, we don't change the method call | ||
| if (logLevelName == null) { | ||
| return m; | ||
| } | ||
|
|
||
| JavaType.Method updatedMethodType = requireNonNull(m.getMethodType()) | ||
| .withParameterTypes(ListUtils.filter(m.getMethodType().getParameterTypes(), it -> !TypeUtils.isAssignableTo("org.jboss.logging.Logger.Level", it))) | ||
| .withParameterNames(ListUtils.filter(m.getMethodType().getParameterNames(), it -> !"level".equals(it))) | ||
| .withName(logLevelName.toLowerCase()); | ||
|
|
||
| return m | ||
| .withArguments(updatedArguments) | ||
| .withMethodType(updatedMethodType) | ||
| .withName(m.getName().withSimpleName(logLevelName.toLowerCase())); | ||
| } | ||
|
|
||
| @Nullable | ||
| String extractLogLevelName(Expression expression) { | ||
| if (expression instanceof J.Identifier) { | ||
| J.Identifier identifier = (J.Identifier) expression; | ||
| if (identifier.getFieldType() != null && | ||
| identifier.getFieldType().getOwner() instanceof JavaType.Class) { | ||
| return identifier.getSimpleName(); | ||
| } | ||
| } else if (expression instanceof J.FieldAccess) { | ||
| return ((J.FieldAccess) expression).getSimpleName(); | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /* | ||
| * Copyright 2024 the original author or authors. | ||
| * <p> | ||
| * Licensed under the Moderne Source Available License (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * <p> | ||
| * https://docs.moderne.io/licensing/moderne-source-available-license | ||
| * <p> | ||
| * 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. | ||
| */ | ||
| @NullMarked | ||
| @NonNullFields | ||
| package org.openrewrite.java.logging.jboss; | ||
|
|
||
| import org.jspecify.annotations.NullMarked; | ||
| import org.openrewrite.internal.lang.NonNullFields; |
Uh oh!
There was an error while loading. Please reload this page.