[all] Use slf4j and SimpleLogger by adangel · Pull Request #3789 · pmd/pmd

adangel · 2022-02-17T18:10:01Z

Describe the PR

Fixes [all] Use slf4j #896
Switching default debug level via CLI option "--debug" is supported. The solution need reflection though (Slf4jSimpleConfiguration) in order to not have a dependency on a specific logging implementation.
The ant integration is also working, but a bit hacky (Slf4jSimpleConfigurationForAnt). It basically intercepts the printstream that SimpleLogger is using. The old classes under net.sourceforge.pmd.util.logger are removed now (they were now unused).
pmd-dist provides the logger implementation, which is slf4j-simple. It also provides the default configuration (simplelogger.properties)
apex used to bring in logback, but it doesn't seem to be used (and luckily it's old and doesn't use slf4j 2.x ServiceLoader mechnism, but the old StaticBinder way - otherwise there would have been two logging implementations...). Maybe we should remove this dependency on master as well...
Only net.sourceforge.pmd.PMD is now migrated to use slf4j

Open Tasks:

Maybe share some code between Slf4jSimpleConfiguration and Slf4jSimpleConfigurationForAnt
Migrate the remaining classes / modules - see [all] Use slf4j (Part 2) #3794
Enable the new rule DoNotUseJavaUtilLogging with priority 1
Add documentation maybe? For rule implementers it would be nice to know how to log.
- also the file "simplelogger.properties" should be explained
- see [all] Use slf4j (Part 2) #3794
Release notes - see [all] Use slf4j (Part 2) #3794

Ready?

Added unit tests for fixed bug/feature
Passing all unit tests
Complete build ./mvnw clean verify passes (checked automatically by github actions)
Added (in-code) documentation (if needed)

Support "--debug" flag for slf4j-simple

This is a hacky way: It reconfigures SimpleLogger to use a special print stream. What ever is printed there is forwarded to ant's project logger. Removed the deprecated and now unused classes in net.sourceforge.pmd.util.log.

ghost · 2022-02-18T14:09:47Z

	2 Messages
📖	Compared to pmd/7.0.x: This changeset changes 32 violations, introduces 31 new violations, 0 new errors and 0 new configuration errors, removes 37 violations, 0 errors and 0 configuration errors. Full report
📖	Compared to master: This changeset changes 57268 violations, introduces 34622 new violations, 7 new errors and 0 new configuration errors, removes 138164 violations, 25 errors and 3 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 29 violations, introduces 19 new violations, 0 new errors and 0 new configuration errors, removes 27 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 57266 violations, introduces 34622 new violations, 7 new errors and 0 new configuration errors, removes 138166 violations, 25 errors and 3 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 49302 violations, introduces 26826 new violations, 5 new errors and 0 new configuration errors, removes 146082 violations, 25 errors and 3 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 0 violations, introduces 0 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 49302 violations, introduces 26828 new violations, 5 new errors and 0 new configuration errors, removes 146082 violations, 25 errors and 3 configuration errors. Full report

Generated by 🚫 Danger

This is at least needed for apex jorje, but might be handy for any other library that logs through jul.

Need to remove all handlers first, as java by default adds a ConsoleLogger. The SLF4JBridgeHandler should be the only handler registered.

oowekyala

Very cool!

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

adangel · 2022-02-19T09:59:54Z

@oowekyala: I'll merge in the other PR (#3794) into this one - as it will only build as a whole. Then this PR will get a bit big unfortunately...

oowekyala

Thanks for the large effort @adangel. I'm not sure what the best way to proceed is wrt conflicts with #3785. I'm inclined to merge this PR first, as it looks ready, and edit #3785 to minimize conflicts (but there will be some). Maybe I should prepare the merge into 7.0.x in a new PR

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

This theoretically allows to use "-Dorg.slf4j.simpleLogger.defaultLogLevel=trace" manually Still log the current default log level (verified in BinaryDistributionIT)

otherwise debug log level is enabled for later tests

adangel · 2022-02-26T12:44:48Z

Thanks for the large effort @adangel. I'm not sure what the best way to proceed is wrt conflicts with #3785. I'm inclined to merge this PR first, as it looks ready, and edit #3785 to minimize conflicts (but there will be some). Maybe I should prepare the merge into 7.0.x in a new PR

Sounds like a plan. The PmdLogger in #3785 is not really needed on master, so I'd suggest to remove it there and add it on 7.0.x after the merge.

Note: Currently in the pmd-java build in the unit tests, something is setting the default log level to DEBUG at some point and doesn't reset it. I see a lot of [main] DEBUG net.sourceforge.pmd.util.ClasspathClassLoader - Adding classpath entry: <.> loggings.... I didn't find the place yet, where this happens

This provides a default configuration that will be used when resetting the logger configuration in CLITests or PMDTaskTest. Without this, slf4j-simple will keep the last configured default level. Now it will use the default level configured in simplelogger. Since pmd-test is not part of the distribution, this configuration file has no effect for the binaries.

adangel · 2022-02-26T19:45:53Z

Note: Currently in the pmd-java build in the unit tests, something is setting the default log level to DEBUG at some point and doesn't reset it. I see a lot of [main] DEBUG net.sourceforge.pmd.util.ClasspathClassLoader - Adding classpath entry: <.> loggings.... I didn't find the place yet, where this happens

I solved it now by adding a default simplelogger.properties in pmd-test. This will be used, when we reset the configuration after a test, that messed with the configuration (to verify, that the --debug flag actually works). That way, it will now reset to whatever is configured in simplelogger.properties.

Refs #3789

adangel added 7 commits February 17, 2022 18:54

[core] Use slf4j-api and slf4j-simple

e1d66e2

Support "--debug" flag for slf4j-simple

[dist] Add default simplelogger.properties

5c7af91

[core] Forward slf4j logging to ant

bc6806d

This is a hacky way: It reconfigures SimpleLogger to use a special print stream. What ever is printed there is forwarded to ant's project logger. Removed the deprecated and now unused classes in net.sourceforge.pmd.util.log.

[dist] Adjust default logging configuration

ad86896

[dist] Add integration test for --debug flag

c166613

[apex] Remove unused logback impl

a8daccb

Use new dogfood rule "DoNotUseJavaUtilLogging"

64a8b37

adangel added this to the 7.0.0 milestone Feb 17, 2022

adangel linked an issue Feb 17, 2022 that may be closed by this pull request

[all] Use slf4j #896

Closed

8 tasks

adangel added 2 commits February 18, 2022 14:22

[core] Refactor Slf4jSimpleConfigurationForAnt

cfbb14c

Merge branch 'pmd/7.0.x' into pmd7-896-slf4j

f6378d1

adangel added 9 commits February 18, 2022 16:03

[core] Use slf4j

9ba34fe

[test] Supress JavaUtilLoggingRule for now

d1cc05e

[core] Add jul-to-slf4j bridge

7b702bd

This is at least needed for apex jorje, but might be handy for any other library that logs through jul.

[apex] Use slf4j

744817a

[java] Use slf4j

138296b

[plsql] Use slf4j

0c0b0ff

[visualforce] Use slf4j

958be69

[core] Fix jul-to-slf4j integration

462d303

Need to remove all handlers first, as java by default adds a ConsoleLogger. The SLF4JBridgeHandler should be the only handler registered.

[doc] Use slf4j

06363db

adangel mentioned this pull request Feb 18, 2022

[all] Use slf4j (Part 2) #3794

Closed

adangel added 2 commits February 18, 2022 19:35

[doc] Update release notes, add logging doc (pmd#896)

4e82462

[doc] Update cli doc for slf4j logging

1d78dba

adangel added a commit to pmd/build-tools that referenced this pull request Feb 18, 2022

Set priority of DoNotUseJavaUtilLogging to 1 (pmd/pmd#3789 pmd/pmd#896)

fcfb446

adangel marked this pull request as ready for review February 18, 2022 18:46

oowekyala reviewed Feb 18, 2022

View reviewed changes

adangel and others added 2 commits February 19, 2022 10:44

Merge branch 'pmd/7.0.x' into pmd7-896-slf4j

c2f99dc

Apply suggestions from code review

e595211

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

adangel added 5 commits February 19, 2022 11:01

Merge branch 'pr-3789' into pmd7-896-slf4j

5a90edc

Merge branch 'pmd7-896-slf4j-migrations' into pmd7-896-slf4j

a592621

[core] Check if SimpleLogger is in use before trying to reconfigure it

c2fd65c

Fix compilation errors

1a0a2d3

Fix tests

868a664

adangel mentioned this pull request Feb 19, 2022

PMD 7 Miscellaneous Tasks #2524

Closed

6 tasks

oowekyala reviewed Feb 25, 2022

View reviewed changes

Comment thread pmd-dist/src/main/resources/scripts/run.sh Outdated

Comment thread pmd-dist/src/main/resources/scripts/run.sh Outdated

Comment thread pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java Outdated

adangel and others added 5 commits February 26, 2022 11:29

Merge branch 'pmd/7.0.x' into pmd7-896-slf4j

2091c78

Apply suggestions from code review

fd6f521

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>

Only configure default loglevel with --debug is used at cli

9f9ceab

This theoretically allows to use "-Dorg.slf4j.simpleLogger.defaultLogLevel=trace" manually Still log the current default log level (verified in BinaryDistributionIT)

Reset logging after PMDTaskTest and (Core)CLITest

56bf780

otherwise debug log level is enabled for later tests

Use Level.valueOf for converting PMD_DEBUG_LEVEL into slf4j Level

e964297

adangel added 3 commits February 26, 2022 14:20

Fix getDefaultLogLevel

fb14974

Silence out pushd/popd

3b7e775

oowekyala added a commit that referenced this pull request Feb 27, 2022

Merge branch 'pr/3789' into 7.0.x

8457e19

Refs #3789

oowekyala merged commit 7a0b20a into pmd:pmd/7.0.x Feb 27, 2022

adangel deleted the pmd7-896-slf4j branch February 28, 2022 18:29

adangel mentioned this pull request Jun 24, 2022

Downgrade slf4j to 1.7.36 #4023

Merged

4 tasks

This was referenced Jan 13, 2023

[all] Use slf4j #896

Closed

[all] Breaking API changes for 7.0.0 #881

Closed

adangel mentioned this pull request Jan 23, 2023

PMD 7 Tracking Issue #3898

Closed

55 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[all] Use slf4j and SimpleLogger#3789

[all] Use slf4j and SimpleLogger#3789
oowekyala merged 35 commits into
pmd:pmd/7.0.xfrom
adangel:pmd7-896-slf4j

adangel commented Feb 17, 2022 •

edited

Loading

Uh oh!

ghost commented Feb 18, 2022 •

edited by ghost

Loading

Uh oh!

oowekyala left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel commented Feb 19, 2022

Uh oh!

oowekyala left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel commented Feb 26, 2022

Uh oh!

adangel commented Feb 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

adangel commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Ready?

Uh oh!

ghost commented Feb 18, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oowekyala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel commented Feb 19, 2022

Uh oh!

oowekyala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel commented Feb 26, 2022

Uh oh!

adangel commented Feb 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adangel commented Feb 17, 2022 •

edited

Loading

ghost commented Feb 18, 2022 •

edited by ghost

Loading