Cherry-pick recent 5.2-compatible changes into the 5.2 branch. #194
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the author inserts a newline between the `import` token and the imported path, that newline shouldn't be considered when sorting the paths.
This ensures that the test function always exists, even if the symbol is not defined, so that `--generate-linuxmain` contains the symbol in both cases. When the symbol is undefined, the empty test will vacuously succeed.
The transform is intentionally meant to only apply to comments that are documenting a declaration. The distinction is made by finding the comment "closest" to the decl, searching through the trivia backwards. Unfortunately, the search didn't handle the fact that there could be multiple doc comments in the leading trivia of a decl. Multiple comments usually occur at the beginning of the file, when there's a file level comment (either doc block or doc line), followed by decl documentation, then the first decl. It might make sense to convert doc-block comments in the file preamble to doc-line, but we've previously had issues converting general purpose block comments. This approach is safer because only comments that are documenting a decl (defined as closest to the decl) are converted.
Standard output does not support `fsync`, which is the system call that this boils down to on Linux (and presumably on Darwin as well). In both cases, `fsync` sets `errno` to `EINVAL`, but unfortunately Foundation's behavior differs per platform: on Darwin the error is silently swallowed, but on Linux it traps.
- Don't insert a space before `where` if it's the first thing in the attribute. - Apply formatting to comma-delimited parameter "tuples" following the `wrt:` clause. Fixes SR-12414.
Some statement types that allow a label were lacking the space token after the label. This caused the formatter to remove spaces when it encountered those stmt types.
…t param. When enum cases have associated values, these are similar to function params. The right paren is allowed to stay on the same line for function params, and this makes enum cases consistent. The break allows discretionary line breaks.
- Handle `()` in closure signature return types. - Handle `()` when nested in the argument lists of function types and closures. - Ensure leading/trailing trivia of the original `()` is preserved. - Don't replace `()` if it has internal comments, like `(/*foo*/)`.
Instead of a bunch of free functions that pass their arguments around, this change now refactors the core format and lint file handling functionality into a set of "frontend" classes so that the diagnostic engine and related state can be created in one place and shared. This also significantly improves error handling. Whereas we were previously killing the process on the first sign of a source file or configuration file being invalid, we now emit an error and skip the file, allowing any remaining inputs to still be processed. Lastly, this introduces a configuration cache so that we're not re-reading the configuration from the file system for every source file we process. In most cases, when we're processing a recursive directory structure, they'll all share a configuration file at a common root, so we cache that based on its file URL and return it when requested.
The implementation of trailing commas, using a workaround of the length calculation algorithm, resulted in edge cases where the trailing comma could overflow the max line length. This edge case happens for elements that should break internally to create additional space for the trailing comma (e.g. arrays, dictionaries, tuples). For example, an array that contains arrays could have its final element overflow the line length if the last element + correct indentation is exactly `maxLineLength`. After the trailing comma is added, the last array element *should* have broken up onto multiple lines to make space for the comma. The problem happens because the comma token was being added at the wrong syntax level, outside of the element's group, and its length wasn't propagating. Not propagating the comma's length was intentional to work around the fact that the trailing comma is only present when the comma delimited collection uses multiple lines. Unfortunately, moving the comma deeper into the element so that it can cause elements to break internally means its length has to be propagate as a normal token to also cause breaks at different syntax levels to fire too. - Array/dictionary elements won't be allowed to overflow the configured line length. - Arrays/dictionaries that could fit on 1 line without a trailing comma will be forced onto multiple lines with a trailing comma *if* they fit exactly at `maxLineLength` (without a trailing comma).
The inheritance list for type and extension decls is wrapped with open/close tokens, but the relevant open-break token was nested inside of a different group. The previous token layout created the correct indentation of the tokens in the inheritance list, but meant the break between the colon and first token in the inheritance list could never fire because it was followed by a close token. This resulted in type and extension decls that overflow the column limit when the type name + `:` + first token in the inhertance list together was longer than the column limit. I fixed this by moving the open break inside of the open/close tokens surrounding the inheritance list. Normally, the open break comes before the open but that would force a break before the first token in the inheritance list if the entire list doesn't fit. That behavior wouldn't be consistent with existing behavior. Instead, placing the open break inside of the open only breaks if the first token in the inheritance list is too long.
This rule wasn't being applied recursively, which meant it wouldn't be applied to the trailing closure of a function call where parens where removed.
The math for calculating how many blank lines to allow didn't account for existing consecutive blank lines when the maximum blank lines configuration value was used. That meant an existing newline in the output would result in allowing max blank lines + 1 blank lines.
swiftlang/swift#30001 removed the logic from the parser to handle the `jvp:` and `vjp:` arguments of the attribute (but left the syntax definitions in place for the time being). Since this causes an attribute with those arguments to fail to parse, I've removed them from the tests so that those tests continue to pass under the new behavior. (The arguments were always optional, so they pass under the old behavior as well.) This also caught and fixed a bug where an attribute with only a `wrt:` and a `where` clause wasn't getting formatted correctly.
This aligns the behavior of while-stmt and if-stmt conditions. The while-stmt conditions didn't have open-breaks, which meant there was no additional indentation nor would continuation breaks be able to stack. This can result in exceeding the max line length if a while-stmt has a label and first condition, up to its first break, that exceeds the remaining line length.
The consistent group forces the bodies of if-stmts with else clauses to have breaks around all braces when the stmt spans multiple lines. Previously. the breaks around the braces only fired if the body didn't fit on the line. That behavior lead to hard to read if-stmts because if was unclear where conditions stopped and the body started.
There's no reason to have a newline between the square brackets when there's nothing, other than a `:` for dict exprs, between the brackets.
The behavior was inconsistent between subscripts and function calls. They both use a same-break that ignores discretionary newlines now. The break ignores discretionary because breaking before a trailing closure's left brace uses excessive vertical whitespace without improving readability. Now the brace is usually glued to the right paren/right square bracket, except when it doesn't fit due to line length.
Using `@derivative(of:...)` without a `wrt` clause previously crashed the formatter, due to an unmatched open token. Now the close token is added when there is no `wrt` clause too.
This test case exercises the case where an open break occurs adjacent to a reset break when the reset break fires. This verifies that the open breaks stack indentation when they occur on the same line, but one of the breaks is at the start of the line.
The implementation of consistent groups forces breaking inside of the group if the group starts at the beginning of a line (i.e. was immediately preceded by a break that fired). That isn't exactly what we want for if-stmt because the stmt generally starts at the beginning of a line and we want the breaks to fire only if the group is longer than the rest of the line. Moving the open token past the if `syntax` token resolves 2 known complexities with consistent groups: 1. Whether an immediately preceding break fired influences 2. The `spaceRemaining` value is only updated after printing the first `syntax` token on a new line It's a little odd to group in this way, since it logically makes more sense to group around the entire if-stmt but there aren't any breaks between the if token and the first condition so moving the open token doesn't change the length of any breaks throughout the statement.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested with
swift testagainst Xcode 11.4/Swift 5.2.