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

Drops Java <16 constraint from pretty_format_kotlin #123

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
os: [macos-latest, ubuntu-latest, windows-latest]

golang_version: [1.19.3]
java_version: ['15', '16', '17']
java_version: ['15', '16', '17', '18', '19']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I removed the test annotations, I'd also like to show that the tool works with even more recent Java versions than 17

python_version: ['3.8', '3.9', '3.10', '3.11']
rust_version: [stable]

Expand Down
28 changes: 16 additions & 12 deletions language_formatters_pre_commit_hooks/pretty_format_kotlin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
import sys
import typing

from packaging.version import Version

from language_formatters_pre_commit_hooks import _get_default_version
from language_formatters_pre_commit_hooks.pre_conditions import assert_max_jdk_version
from language_formatters_pre_commit_hooks.pre_conditions import java_required
from language_formatters_pre_commit_hooks.utils import download_url
from language_formatters_pre_commit_hooks.utils import run_command
Expand Down Expand Up @@ -58,30 +55,37 @@ def pretty_format_kotlin(argv: typing.Optional[typing.List[str]] = None) -> int:
parser.add_argument("filenames", nargs="*", help="Filenames to fix")
args = parser.parse_args(argv)

# KTLint does not yet support Java 16+, before that version.
# Let's make sure that we report a nice error message instead of a complex
# Java Stacktrace
# the tool can only be executed on Java up to version 15.
# Context: https://github.com/JLLeitschuh/ktlint-gradle/issues/461
assert_max_jdk_version(Version("16.0"), inclusive=False) # pragma: no cover
macisamuele marked this conversation as resolved.
Show resolved Hide resolved

ktlint_jar = _download_kotlin_formatter_jar(
args.ktlint_version,
)

jvm_args = ["--add-opens", "java.base/java.lang=ALL-UNNAMED"]

# ktlint does not return exit-code!=0 if we're formatting them.
# To workaround this limitation we do run ktlint in check mode only,
# which provides the expected exit status and we run it again in format
# mode if autofix flag is enabled
check_status, check_output = run_command("java", "-jar", ktlint_jar, "--verbose", "--relative", "--", *_fix_paths(args.filenames))
check_status, check_output = run_command(
"java", *jvm_args, "-jar", ktlint_jar, "--verbose", "--relative", "--", *_fix_paths(args.filenames)
)

not_pretty_formatted_files: typing.Set[str] = set()
if check_status != 0:
not_pretty_formatted_files.update(line.split(":", 1)[0] for line in check_output.splitlines())

if args.autofix:
print("Running ktlint format on {}".format(not_pretty_formatted_files))
run_command("java", "-jar", ktlint_jar, "--verbose", "--relative", "--format", "--", *_fix_paths(not_pretty_formatted_files))
run_command(
"java",
*jvm_args,
"-jar",
ktlint_jar,
"--verbose",
"--relative",
"--format",
"--",
*_fix_paths(not_pretty_formatted_files),
)

status = 0
if not_pretty_formatted_files:
Expand Down
22 changes: 0 additions & 22 deletions tests/pretty_format_kotlin_test.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
# -*- coding: utf-8 -*-
from unittest.mock import patch

import pytest
from packaging.version import Version

from language_formatters_pre_commit_hooks import _get_default_version
from language_formatters_pre_commit_hooks.pre_conditions import get_jdk_version
from language_formatters_pre_commit_hooks.pretty_format_kotlin import _download_kotlin_formatter_jar
from language_formatters_pre_commit_hooks.pretty_format_kotlin import pretty_format_kotlin
from tests import change_dir_context
from tests import run_autofix_test
from tests import undecorate_function
from tests import UnexpectedStatusCode


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -62,26 +57,9 @@ def test__download_kotlin_formatter_jar(ensure_download_possible, version): # n
("NotPrettyFormattedFixed.kt", 0),
),
)
@pytest.mark.skipif(condition=get_jdk_version() >= Version("16"), reason="Skipping test because it requires Java JDK lower than 16")
def test_pretty_format_kotlin(undecorate_method, filename, expected_retval):
assert undecorate_method([filename]) == expected_retval


@pytest.mark.skipif(condition=get_jdk_version() >= Version("16"), reason="Skipping test because it requires Java JDK lower than 16")
def test_pretty_format_kotlin_autofix(tmpdir, undecorate_method):
run_autofix_test(tmpdir, undecorate_method, "NotPrettyFormatted.kt", "NotPrettyFormattedFixed.kt")


@pytest.mark.skipif(condition=get_jdk_version() < Version("16"), reason="Skipping test because it requires Java JDK 16+")
def test_ktlint_does_not_yet_support_java_16_or_more(tmpdir, undecorate_method):
macisamuele marked this conversation as resolved.
Show resolved Hide resolved
"""
Test that running pretty-format-kotlin with Java 16+ is still not supported.
The test is meant to be an early indicator that Java 16+ is now supported and as such
`assert_max_jdk_version(...)` invocation can be removed from `pretty_format_kotlin.py` file
"""
with patch(
"language_formatters_pre_commit_hooks.pretty_format_kotlin.assert_max_jdk_version",
autospec=True,
return_value=None,
), pytest.raises(UnexpectedStatusCode):
run_autofix_test(tmpdir, undecorate_method, "NotPrettyFormatted.kt", "NotPrettyFormattedFixed.kt")