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

Add support for ktfmt #196

Merged
merged 8 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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 .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
minimum_pre_commit_version: '1'
- id: pretty-format-kotlin
name: KTLint
description: Runs KTLint over Kotlin source files
description: Runs KTLint (or ktfmt) over Kotlin source files
entry: pretty-format-kotlin
language: python
types: [kotlin]
Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ You can pass the jar file path to `--google-java-formatter-jar` argument:
args: [--google-java-formatter-jar=/usr/bin/google-java-format-1.17.0-all-deps.jar]
```

### How to use ktfmt instead of ktlint

```yaml
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: ...
hooks:
- id: pretty-format-kotlin
args: [--ktfmt --ktfmt-style=google]
```

Supported styles are google (default), dropbox and kotlinlang

## License

`language-formatters-pre-commit-hooks` is licensed with [`Apache License version 2.0`](http://www.apache.org/licenses/LICENSE-2.0.html).
1 change: 1 addition & 0 deletions language_formatters_pre_commit_hooks/ktfmt.version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0.46
66 changes: 58 additions & 8 deletions language_formatters_pre_commit_hooks/pretty_format_kotlin.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from language_formatters_pre_commit_hooks.utils import run_command


def _download_kotlin_formatter_jar(version: str) -> str: # pragma: no cover
def _download_ktlint_formatter_jar(version: str) -> str: # pragma: no cover
def get_url(_version: str) -> str:
# Links extracted from https://github.com/pinterest/ktlint/
return "https://github.com/pinterest/ktlint/releases/download/{version}/ktlint".format(
Expand All @@ -30,6 +30,18 @@ def get_url(_version: str) -> str:
)


def _download_ktfmt_formatter_jar(version: str) -> str: # pragma: no cover
url = "https://repo1.maven.org/maven2/com/facebook/ktfmt/{version}/ktfmt-{version}-jar-with-dependencies.jar".format(
version=version,
)
try:
return download_url(url)
except: # noqa: E722 (allow usage of bare 'except')
raise RuntimeError(
f"Failed to download {url}. Probably the requested version, {version}" ", is not valid or you have some network issue."
)


def _fix_paths(paths: typing.Iterable[str]) -> typing.Iterable[str]:
# Starting from KTLint 0.41.0 paths cannot contain backward slashes as path separator
# Odd enough the error messages reported by KTLint contain `\` :(
Expand All @@ -52,14 +64,52 @@ def pretty_format_kotlin(argv: typing.Optional[typing.List[str]] = None) -> int:
default=_get_default_version("ktlint"),
help="KTLint version to use (default %(default)s)",
)

parser.add_argument(
Copy link
Owner

Choose a reason for hiding this comment

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

Need to think a little bit about the arguments, the PR adds 2 arguments that would only be honoured if ktfmt flag is passed.
As of now I don't have a much better option, hence not a blocker at all.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I guess what's important is that autofix works in the same way for both ktlint and ktfmt. I'm not sure we can do much about therest.

"--ktfmt-version",
dest="kftmt_version",
default=_get_default_version("ktfmt"),
help="ktfmt version to use (default %(default)s)",
)
parser.add_argument(
"--ktfmt",
action="store_true",
dest="ktfmt",
help="Use ktfmt",
)
parser.add_argument(
"--ktfmt-style",
choices=["dropbox", "google", "kotlinlang"],
dest="ktfmt_style",
help="Which style to use",
)
parser.add_argument("filenames", nargs="*", help="Filenames to fix")
args = parser.parse_args(argv)

ktlint_jar = _download_kotlin_formatter_jar(
args.ktlint_version,
if args.ktfmt:
return run_ktfmt(args.kftmt_version, args.filenames, args.ktfmt_style, args.autofix)
else:
return run_ktlint(args.ktlint_version, args.filenames, args.autofix)


def run_ktfmt(ktfmt_version: str, filenames: typing.Iterable[str], ktfmt_style: typing.Optional[str], autofix: bool) -> int:
jar = _download_ktfmt_formatter_jar(ktfmt_version)
ktfmt_args = ["--set-exit-if-changed"]
if ktfmt_style is not None:
ktfmt_args.append(f"--{ktfmt_style}-style")
if not autofix:
ktfmt_args.append("--dry-run")
return_code, _, _ = run_command(
"java",
"-jar",
jar,
"ktfmt",
*ktfmt_args,
*filenames,
)
return return_code


def run_ktlint(ktlint_version: str, filenames: typing.Iterable[str], autofix: bool):
ktlint_jar = _download_ktlint_formatter_jar(ktlint_version)
jvm_args = ["--add-opens", "java.base/java.lang=ALL-UNNAMED"]

# ktlint does not return exit-code!=0 if we're formatting them.
Expand All @@ -79,14 +129,14 @@ def pretty_format_kotlin(argv: typing.Optional[typing.List[str]] = None) -> int:
"--reporter=json",
"--relative",
"--",
*_fix_paths(args.filenames),
*_fix_paths(filenames),
)

not_pretty_formatted_files: typing.Set[str] = set()
if return_code != 0:
not_pretty_formatted_files.update(item["file"] for item in json.loads(stdout))

if args.autofix:
if autofix:
print("Running ktlint format on {}".format(not_pretty_formatted_files))
run_command(
"java",
Expand All @@ -106,7 +156,7 @@ def pretty_format_kotlin(argv: typing.Optional[typing.List[str]] = None) -> int:
status = 1
print(
"{}: {}".format(
"The following files have been fixed by ktlint" if args.autofix else "The following files are not properly formatted",
"The following files have been fixed by ktlint" if autofix else "The following files are not properly formatted",
", ".join(sorted(not_pretty_formatted_files)),
),
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fun main(args: Array<String>) {
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for adding the end-to-end tests and examples.

I'm wondering if it would be better to have the ktfmt fixtures in a separate directories.
Possibly we could have the following fixtures paths

  • test-data/pretty_format_kotlin/ktlint <- move in there all the pre-PR fixture
  • test-data/pretty_format_kotlin/ktfmt <- add in here all the new fixtures related to ktfmt

Copy link
Author

Choose a reason for hiding this comment

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

Not too sure about this one, since we share some files between the two tests. Also I'd have to change some of the helper code to look in different directory based on the formatter.

I'm happy to do it if you really insist.

println("Hello, world!")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fun main(args: Array<String>) {
println("Hello, world!")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fun main(args: Array<String>) {
println("Hello, world!")
}
6 changes: 4 additions & 2 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ def run_autofix_test(
method: typing.Callable[[typing.List[str]], int],
not_pretty_formatted_path: str,
formatted_path: str,
extra_parameters: typing.Optional[typing.List[str]] = None,
) -> None:
extra_parameters = [] if extra_parameters is None else extra_parameters
tmpdir.mkdir("src")
not_pretty_formatted_tmp_path = tmpdir.join("src").join(basename(not_pretty_formatted_path))

Expand All @@ -55,14 +57,14 @@ def run_autofix_test(

copyfile(not_pretty_formatted_path, not_pretty_formatted_tmp_path)
with change_dir_context(tmpdir.strpath):
parameters = ["--autofix", not_pretty_formatted_tmp_strpath]
parameters = extra_parameters + ["--autofix"] + [not_pretty_formatted_tmp_strpath]
status_code = method(parameters)
if status_code != 1:
raise UnexpectedStatusCode(parameters=parameters, expected_status_code=1, actual_status_code=status_code)

# file was formatted (shouldn't trigger linter again)
with change_dir_context(tmpdir.strpath):
parameters = ["--autofix", not_pretty_formatted_tmp_strpath]
parameters = extra_parameters + ["--autofix", not_pretty_formatted_tmp_strpath]
status_code = method(parameters)
if status_code != 0:
raise UnexpectedStatusCode(parameters=parameters, expected_status_code=0, actual_status_code=status_code)
Expand Down
94 changes: 88 additions & 6 deletions tests/pretty_format_kotlin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
import pytest

from language_formatters_pre_commit_hooks import _get_default_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 _download_ktfmt_formatter_jar
from language_formatters_pre_commit_hooks.pretty_format_kotlin import _download_ktlint_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
Expand Down Expand Up @@ -44,8 +45,23 @@ def undecorate_method():
),
)
@pytest.mark.integration
def test__download_kotlin_formatter_jar(ensure_download_possible, version): # noqa: F811
_download_kotlin_formatter_jar(version)
def test__download_kotlin_formatter_jar_ktlint(ensure_download_possible, version): # noqa: F811
_download_ktlint_formatter_jar(version)


@pytest.mark.parametrize(
"version",
sorted(
{
_get_default_version("ktfmt"),
"0.45",
"0.46",
}
),
)
@pytest.mark.integration
def test__download_kotlin_formatter_jar_ktfmt(ensure_download_possible, version): # noqa: F811
_download_ktfmt_formatter_jar(version)


@pytest.mark.parametrize(
Expand All @@ -57,9 +73,75 @@ def test__download_kotlin_formatter_jar(ensure_download_possible, version): # n
("NotPrettyFormattedFixed.kt", 0),
),
)
def test_pretty_format_kotlin(undecorate_method, filename, expected_retval):
def test_pretty_format_kotlin_ktlint(undecorate_method, filename, expected_retval):
assert undecorate_method([filename]) == expected_retval


def test_pretty_format_kotlin_autofix(tmpdir, undecorate_method):
run_autofix_test(tmpdir, undecorate_method, "NotPrettyFormatted.kt", "NotPrettyFormattedFixed.kt")
@pytest.mark.parametrize(
("filename", "expected_retval"),
(
("NotPrettyFormattedFixedKtfmtDropbox.kt", 1),
("NotPrettyFormattedFixedKtfmtGoogle.kt", 0),
("NotPrettyFormattedFixedKtfmtKotlinlang.kt", 1),
("NotPrettyFormatted.kt", 1),
("Invalid.kt", 1),
),
)
def test_pretty_format_kotlin_ktfmt(undecorate_method, filename, expected_retval):
assert undecorate_method(["--ktfmt", filename]) == expected_retval


@pytest.mark.parametrize(
("filename", "expected_retval"),
(
("NotPrettyFormattedFixedKtfmtDropbox.kt", 0),
("NotPrettyFormattedFixedKtfmtGoogle.kt", 1),
("NotPrettyFormattedFixedKtfmtKotlinlang.kt", 0),
("NotPrettyFormatted.kt", 1),
("Invalid.kt", 1),
),
)
def test_pretty_format_kotlin_ktfmt_dropbox_style(undecorate_method, filename, expected_retval):
assert undecorate_method(["--ktfmt", "--ktfmt-style=dropbox", filename]) == expected_retval


@pytest.mark.parametrize(
("filename", "expected_retval"),
(
("NotPrettyFormattedFixedKtfmtDropbox.kt", 1),
("NotPrettyFormattedFixedKtfmtGoogle.kt", 0),
("NotPrettyFormattedFixedKtfmtKotlinlang.kt", 1),
("NotPrettyFormatted.kt", 1),
("Invalid.kt", 1),
),
)
def test_pretty_format_kotlin_ktfmt_google_style(undecorate_method, filename, expected_retval):
assert undecorate_method(["--ktfmt", "--ktfmt-style=google", filename]) == expected_retval


@pytest.mark.parametrize(
("filename", "expected_retval"),
(
("NotPrettyFormattedFixedKtfmtDropbox.kt", 0),
("NotPrettyFormattedFixedKtfmtGoogle.kt", 1),
("NotPrettyFormattedFixedKtfmtKotlinlang.kt", 0),
("NotPrettyFormatted.kt", 1),
("Invalid.kt", 1),
),
)
def test_pretty_format_kotlin_ktfmt_kotlinglang_style(undecorate_method, filename, expected_retval):
assert undecorate_method(["--ktfmt", "--ktfmt-style=kotlinlang", filename]) == expected_retval


@pytest.mark.parametrize(
("filename", "fixed_filename", "extra_parameters"),
(
("NotPrettyFormatted.kt", "NotPrettyFormattedFixedKtlint.kt", []),
("NotPrettyFormatted.kt", "NotPrettyFormattedFixedKtfmtGoogle.kt", ["--ktfmt"]),
("NotPrettyFormatted.kt", "NotPrettyFormattedFixedKtfmtGoogle.kt", ["--ktfmt", "--ktfmt-style=google"]),
("NotPrettyFormatted.kt", "NotPrettyFormattedFixedKtfmtDropbox.kt", ["--ktfmt", "--ktfmt-style=dropbox"]),
("NotPrettyFormatted.kt", "NotPrettyFormattedFixedKtfmtKotlinlang.kt", ["--ktfmt", "--ktfmt-style=kotlinlang"]),
),
)
def test_pretty_format_kotlin_autofix(tmpdir, undecorate_method, filename, fixed_filename, extra_parameters):
run_autofix_test(tmpdir, undecorate_method, filename, fixed_filename, extra_parameters)
Loading