Summary

Update Checkstyle for new tool support and improved configuration.

Review Request #11584 — Created April 8, 2021 and submitted June 9, 2021, 1:08 a.m.

Information

Owner

chipx86

Repository

ReviewBot

Branch

release-3.0.x

Bugs

Depends On

Reviewers

Groups

reviewbot

People

Description

This updates CheckstyleTool to subclass the newer BaseTool, and to
use the newer features it offers (dependencies, improved file extension
handling, centralized base command building, new comment options, and
Java tool mixin).

Configuration for checkstyle has improved. It previously required a full
XML file body to be set, which was a lot to manage. Now, to simplify
things, we also allow a Checkstyle-bundled XML file to be specified
(google_checks.xml or sun_checks.xml). If provided, we'll use that
instead of assuming the content is an XML document body.

They could also potentially drop a .jar file in the class path with
XML files, and reference that in the Configuration XML option.

Parsing has improved, handling syntax errors with configurations or
source code and leaving a suitable comment. For other errors, we now
include column, severity, and error code information in the comment.

Python 3 compatibility was also fixed. make_tempfile() was being
called with a Unicode string for the configuration contents, but
requires a byte string.

Testing Done

Unit tests pass on Python 2.7 and 3.x.

Commits

Summary	ID
Update Checkstyle for new tool support and improved configuration. This updates `CheckstyleTool` to subclass the newer `BaseTool`, and to use the newer features it offers (dependencies, improved file extension handling, centralized base command building, new comment options, and Java tool mixin). Configuration for checkstyle has improved. It previously required a full XML file body to be set, which was a lot to manage. Now, to simplify things, we also allow a Checkstyle-bundled XML file to be specified (`google_checks.xml` or `sun_checks.xml`). If provided, we'll use that instead of assuming the content is an XML document body. They could also potentially drop a `.jar` file in the class path with XML files, and reference that in the `Configuration XML` option. Parsing has improved, handling syntax errors with configurations or source code and leaving a suitable comment. For other errors, we now include column, severity, and error code information in the comment. Python 3 compatibility was also fixed. `make_tempfile()` was being called with a Unicode string for the configuration contents, but requires a byte string.	ae6125768fb2d9a73ea1490ff289e771ee161e84

Summary

Update Checkstyle for new tool support and improved configuration.

This updates `CheckstyleTool` to subclass the newer `BaseTool`, and to use the newer features it offers (dependencies, improved file extension handling, centralized base command building, new comment options, and Java tool mixin). Configuration for checkstyle has improved. It previously required a full XML file body to be set, which was a lot to manage. Now, to simplify things, we also allow a Checkstyle-bundled XML file to be specified (`google_checks.xml` or `sun_checks.xml`). If provided, we'll use that instead of assuming the content is an XML document body. They could also potentially drop a `.jar` file in the class path with XML files, and reference that in the `Configuration XML` option. Parsing has improved, handling syntax errors with configurations or source code and leaving a suitable comment. For other errors, we now include column, severity, and error code information in the comment. Python 3 compatibility was also fixed. `make_tempfile()` was being called with a Unicode string for the configuration contents, but requires a byte string.

ae6125768fb2d9a73ea1490ff289e771ee161e84

Issues

Description	From	Last Updated
Leftover debug output?	david	June 2, 2021, 6:29 p.m.
F401 'reviewbot.utils.process.is_exe_in_path' imported but unused	reviewbot	April 8, 2021, 4:13 a.m.

flake8 failed.

JSHint passed.

flake8

bot/reviewbot/tools/tests/test_checkstyle.py (Diff revision 1)
Show all issues
```
F401 'reviewbot.utils.process.is_exe_in_path' imported but unused
```

bot/reviewbot/tools/checkstyle.py (Diff revision 1)
Show all issues
```
Leftover debug output?
```

Change Summary:


Removed leftover debug output.
pycodestyle -> checkstyle in the docstring.

Commits:

	Summary	ID
	Update Checkstyle for new tool support and improved configuration. This updates `CheckstyleTool` to subclass the newer `BaseTool`, and to use the newer features it offers (dependencies, improved file extension handling, centralized base command building, new comment options, and Java tool mixin). Configuration for checkstyle has improved. It previously required a full XML file body to be set, which was a lot to manage. Now, to simplify things, we also allow a Checkstyle-bundled XML file to be specified (`google_checks.xml` or `sun_checks.xml`). If provided, we'll use that instead of assuming the content is an XML document body. They could also potentially drop a `.jar` file in the class path with XML files, and reference that in the `Configuration XML` option. Parsing has improved, handling syntax errors with configurations or source code and leaving a suitable comment. For other errors, we now include column, severity, and error code information in the comment. Python 3 compatibility was also fixed. `make_tempfile()` was being called with a Unicode string for the configuration contents, but requires a byte string.	35f9ee7552f1caf6e154f5ddd75e9a6133fbfa05
	Update Checkstyle for new tool support and improved configuration. This updates `CheckstyleTool` to subclass the newer `BaseTool`, and to use the newer features it offers (dependencies, improved file extension handling, centralized base command building, new comment options, and Java tool mixin). Configuration for checkstyle has improved. It previously required a full XML file body to be set, which was a lot to manage. Now, to simplify things, we also allow a Checkstyle-bundled XML file to be specified (`google_checks.xml` or `sun_checks.xml`). If provided, we'll use that instead of assuming the content is an XML document body. They could also potentially drop a `.jar` file in the class path with XML files, and reference that in the `Configuration XML` option. Parsing has improved, handling syntax errors with configurations or source code and leaving a suitable comment. For other errors, we now include column, severity, and error code information in the comment. Python 3 compatibility was also fixed. `make_tempfile()` was being called with a Unicode string for the configuration contents, but requires a byte string.	ae6125768fb2d9a73ea1490ff289e771ee161e84

Diff:

Revision 2 (+956 -186)

Show changes

	bot/reviewbot/config.py
	bot/reviewbot/tests/test_config.py
	bot/reviewbot/tools/checkstyle.py
	bot/reviewbot/tools/tests/test_checkstyle.py
	docs/reviewbot/upgrading.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (a24cc9a)