Skip to content
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

Added support for logsafe Arg arrays in Slf4jLogsafeArgs. #1394

Merged
merged 13 commits into from
Jun 17, 2020
Merged

Added support for logsafe Arg arrays in Slf4jLogsafeArgs. #1394

merged 13 commits into from
Jun 17, 2020

Conversation

aioobe
Copy link
Contributor

@aioobe aioobe commented Jun 8, 2020

Before this PR

Slf4jLogsafeArgs complains that an Arg<?>[] isn't a logsafe argument.

After this PR

==COMMIT_MSG==
Slf4jLogsafeArgs now allows Arg<?>[] to be passed as vararg argument to logging methods.
==COMMIT_MSG==

Possible downsides?

None.

Context

Only allowing Arg<?>[] gets us half way. Compiler would still give the the following diagnostic warning:

... non-varargs call of varargs method with inexact argument type for last parameter;
        cast to java.lang.Object for a varargs call
        cast to java.lang.Object[] for a non-varargs call and to suppress this warning 

By following the above suggestion and casting to Object[] the expression is no longer of Arg<?>[] type, hence we add support also for Arg<?>[] cast into an Object[].

For // BUG: Diagnostic contains: varargs method with inexact argument to work, we can't only pick up Slf4jLogsafeArgs diagnostics, so we add .matchAllDiagnostics().

With .matchAllDiagnostics() we need to add @SuppressWarnings("deprecation") in order to avoid diagnostics about hasChildren being deprecated.

@policy-bot policy-bot bot requested a review from ferozco June 8, 2020 02:16
@carterkozak carterkozak self-requested a review June 11, 2020 17:58
@carterkozak
Copy link
Contributor

Description is a little out of date now, and it needs a changelog. I'm happy to merge this once those are in :-)

aioobe and others added 4 commits June 12, 2020 12:31
Adds the proper configuration files upon IntelliJ import of a gradle project for checkstyle and copyright.

This generates the following additional files:
- .idea/copyright/profiles_settings.xml
- an xml file under .idea/copyright/ per copyright file under .baseline/copyright
- .idea/checkstyle-idea.xml (and adds Checkstyle-IDEA to the external dependencies) if baseline-checkstyle is applied
- Either .idea/codeStyleSettings.xml or a .idea/codeStyles/ folder with the contents being copied from .baseline/idea
  - If .baseline/idea/codeStyles is present, it will copy its contents, otherwise, it will fall back to .baseline/idea/intellij-java-palantir-style.xml as currently
  - The fallback is using a legacy IntelliJ format and requires closing and reopening the project to be taken into account
@carterkozak
Copy link
Contributor

I think this needs to have develop merged into it to pick up the unit-test-14 check

@ferozco
Copy link
Contributor

ferozco commented Jun 17, 2020

👍

@bulldozer-bot bulldozer-bot bot merged commit d1daefb into palantir:develop Jun 17, 2020
@svc-autorelease
Copy link
Collaborator

Failed to load project - please reach out to #dev-foundry-infra or check Aries to debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants