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

Downgrade editor placeholder in source file from error to warning. #547

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

allevato
Copy link
Member

No tests for this currently, since we don't have tests for the top-level API layer (we should, though). Tested by running the command line tool manually:

swift build && .build/debug/swift-format <<'EOF'
let x = <#value#>
EOF
Building for debugging...
[14/14] Linking swift-format
Build complete! (1.25s)
<stdin>:1:9: warning: editor placeholder in source file

Fixes #526.

@allevato allevato requested a review from ahoppen June 20, 2023 15:51
@allevato
Copy link
Member Author

cc @ahoppen , in case there's a better way to hold these APIs.

Copy link
Contributor

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

LGTM

@HassanTaleb90
Copy link
Contributor

Tested via this code:

try formatter.format(source: str, assumingFileURL: nil, to: &outputStream, parsingDiagnosticHandler: { diagnostic, sourceLocation in
				let severity = diagnostic.diagMessage.severity
				print(severity) //warning
			})
			
			catch SwiftFormatError.fileContainsInvalidSyntax {
				print("Your code contains invalid or unrecognized Swift syntax and cannot be handled safely.")
			} catch {
				print(error)
			}

I guess logically the code must be formatted even with the warnings. (like Xcode does)
But in this way the format causes an error through SwiftFormatError.fileContainsInvalidSyntax

@allevato allevato force-pushed the downgrade-placeholder-errors branch from 0aa33d1 to c71c897 Compare June 26, 2023 12:25
@allevato
Copy link
Member Author

allevato commented Jun 26, 2023

I guess logically the code must be formatted even with the warnings. (like Xcode does)
But in this way the format causes an error through SwiftFormatError.fileContainsInvalidSyntax

Right, I missed the handling of this error during my testing. This should be fixed now. Thanks!

@HassanTaleb90
Copy link
Contributor

HassanTaleb90 commented Jun 26, 2023

There is some situation where is the placeholder throws an error, but I guess the code should be formatted even with these errors. (like Xcode does with Re-Indent)
Example to test:

      func <#name#>(<#parameters#>) -> <#return type#> {
          	<#function body#>
}
               convenience init(<#parameters#>) {
	<#statements#>
}
     init(<#parameters#>) {
	<#statements#>
}
      required init(<#parameters#>) {
             	<#statements#>
}
       {      (<#parameters#>) ->  <#return type#> in
	<#statements#>
}
                func test<#Name#>() {
	<#statements#>
}

Expected results:

func <#name#>(<#parameters#>) -> <#return type#> {
	<#function body#>
}
convenience init(<#parameters#>) {
	<#statements#>
}
init(<#parameters#>) {
	<#statements#>
}
required init(<#parameters#>) {
	<#statements#>
}
{ (<#parameters#>) -> <#return type#> in
	<#statements#>
}
func test<#Name#>() {
	<#statements#>
}

Results on Xcode:
Screenshot 2023-06-26 at 10 49 02 PM

Results using Swift Format on my App:
Simulator Screenshot - iPhone 14 Pro Max - 2023-06-26 at 22 57 18

@allevato
Copy link
Member Author

Those cases would be trickier to handle, since it would require handling individual error cases in very specific ways. For example, the <#parameters#> one looks like this in the syntax tree:

│     │ │ ├─parameterList: FunctionParameterListSyntax
│     │ │ │ ╰─[0]: FunctionParameterSyntax
│     │ │ │   ├─firstName: identifier("x")
│     │ │ │   ├─colon: colon MISSING
│     │ │ │   ╰─type: MissingTypeSyntax
│     │ │ │     ╰─placeholder: identifier("<#type#>") MISSING

so the file as written is syntactically invalid, because a function parameter has to have a colon and a type. We can't just ignore those in the current implementation because we hang certain formatting commands off of individual tokens, some of those commands have to be balanced, and if a token is missing, we'll end up imbalanced.

We can do better here, I think. Since swift-format's algorithm was first written, we can now visit all the tokens (even the missing ones) in the tree, not just the ones that are present in source text. That would remedy the imbalanced-command problem and give us a way to handle cases like this better, and essentially would let us format even arbitrarily invalid code. That's a bigger refactoring though, so I'm not going to try to tackle it in this PR. But it's a great future enhancement.

@allevato allevato merged commit 2480f73 into swiftlang:main Jun 27, 2023
@allevato allevato deleted the downgrade-placeholder-errors branch June 27, 2023 14:06
allevato added a commit to allevato/swift-format that referenced this pull request Jun 29, 2023
…errors

Downgrade `editor placeholder in source file` from error to warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error: editor placeholder in source file
3 participants