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

Ant-style path pattern support broken when ktlint is invoked from the command line #1601

Closed
Tracked by #1397
0x6675636b796f75676974687562 opened this issue Aug 22, 2022 · 4 comments · Fixed by #1615
Labels
cli ktlint command line interface
Milestone

Comments

@0x6675636b796f75676974687562
Copy link
Contributor

Expected Behavior

Judging from its documentation, ktlint should support Ant-style path patterns, as described in

Observed Behavior

Consider I have one or more *.kt files in the src/ subdirectory of the current directory.

  • The invocation below works, the regular shell wildcard gets expanded by any POSIX-compliant shell:
    ktlint src/*.kt
  • Any of these invocations work in zsh (the more complex Ant-style patterns get expanded by the shell itself):
    ktlint **/*.kt
    ktlint ./**/*.kt
    ktlint src/**/*.kt
    ktlint ./src/**/*.kt
    ktlint **/src/*.kt
    ktlint ./**/src/*.kt
    ktlint **/src/**/*.kt
    ktlint ./**/src/**/*.kt

Now, let's enclose the last argument in double quotes so that it gets expanded (as we hope for) by ktlint rather than the underlying shell:

  • ktlint "src/*.kt" — doesn't work (No files matched [src/*.kt]).
  • ktlint "**/*.kt" — works.
  • ktlint "./**/*.kt" — doesn't work.
  • ktlint "src/**/*.kt" — doesn't work (this is actually the example from the output of ktlint --help).
  • ktlint "./src/**/*.kt" — doesn't work.
  • ktlint "**/src/*.kt" — doesn't work.
  • ktlint "./**/src/*.kt" — doesn't work.
  • ktlint "**/src/**/*.kt" — doesn't work.
  • ktlint "./**/src/**/*.kt" — doesn't work.

Steps to Reproduce

Your Environment

  • Version of ktlint used: 0.47.
  • Relevant parts of the .editorconfig settings: none
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): CLI
  • Version of Gradle used (if applicable): N/A
  • Operating System and version:
    • MINGW64_NT-10.0-19043 unit-725 3.3.4-341.x86_64 2022-05-09 11:56 UTC x86_64 Msys
    • Linux unit-725 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 GNU/Linux
@0x6675636b796f75676974687562
Copy link
Contributor Author

An additional problem is that path patterns which ktlint produces automatically, when launched w/o arguments, are platform-specific, i. e. they use backslashes on Windows: #1611.

This is a clear violation of Ant pattern syntax (see the linked pages).

@paul-dingemans
Copy link
Collaborator

Yes, apparently I have just assumed that the path patterns followed the default directory separator of the file system.

internal val defaultPatterns = setOf(
    "**$globSeparator*.kt",
    "**$globSeparator*.kts"
)

...

private val globSeparator: String get() =
    when {
        os.startsWith("windows", ignoreCase = true) -> "\\\\"
        else -> "/"
    }

paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Sep 1, 2022
…matching

Globs always use a "/" as directory separator on all OS's. Input patterns containing
a "\" on Windows OS are transformed to "/" as users on Windows more likely would
assume that the "\" may be used.

On WindowsOS, transform "\" in the filepath to "/" before comparing the filename with
the regular expression (of the glob) which always uses "/" as separator.

Refactor all logic which create globs based on an input path.
- If a path (absolute or relative) point to a directory, that path is expanded
  to the default globs (*.kt, *.kts) in that specific directory or any of its
  subdirectories.
- If a path (absolute or relative) does not point to a directory, e.g. it
  points to a file, or it is a pattern. See "**" replacement below.
- On Windows OS patters containing a "*" (or "**") can not be resolved with
  default Paths utilities. In such case the given input pattern is handled as
  is. See "**" replacement below.

Patterns that contain one or more occurrence of a "**" are split into multiple
patterns so that files on that specific path and subdirectories will be matched.
 - For example, for path "some/path/**/*.kt" an additional pattern
   "some/path/*.kt" is generated to make sure that not only the "*.kt" files in
   a subdirectory of "some/path/" are found but also the "*.kt" in directory
   "some/path" as well. This is in sync with the "**" notation in a glob which
   should be interpreted as having zero or more intermediate subdirectories.
 - For example, for path "some/**/path/**/*.kt", multiple additional patterns
   are generated. As it contains two "**" patterns, 2 x 2 patterns are needed
   to match all possible combinations:
   - "some/**/path/**/*.kt"
   - "some/**/path/*.kt"
   - "some/path/**/*.kt"
   - "some/path/*.kt"

Finally, on Windows OS more fixes are needed as the resulting globs may not
contain any drive destinations as the start of the path. Such a drive
destination is replaced with a "**". So "D:/some/path/*.kt" becomes
"/some/path/*.kt". Note that the last glob representation is less strict than
the original pattern as it could match on other drives that "D:/" as well.

Extend trace logging.

Closes pinterest#1600
Closes pinterest#1601
@paul-dingemans
Copy link
Collaborator

Hi @0x6675636b796f75676974687562 ,

As far as I can see, I have fixed the problem with path traversing (#1600) and the ant-path parsing. I would be grateful if you could test latest snapshot (see https://pinterest.github.io/ktlint/install/snapshot-build/) to see whether the problems are resolved on Windows.

@0x6675636b796f75676974687562
Copy link
Contributor Author

@paul-dingemans, I've tested the latest snapshot on Windows, and all the aforementioned Ant patterns work now.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli ktlint command line interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants