fix for SwiftSyntaxParser changes

[rmaz]

This PR updates the imports to handle the separation of SwiftSyntax and SwiftSyntaxParser:

swiftlang/swift-syntax#309

[rmaz]

@allevato not sure who is the maintainer, could you add a reviewer if you know? thanks

[akyrtzi]

Why is import SwiftSyntaxParser necessary to add in so many files? /cc @ahoppen

[rmaz]

The Diagnostic types were moved to SwiftSyntaxParser, and these are used extensively.

[akyrtzi]

The Diagnostic types were moved to SwiftSyntaxParser, and these are used extensively.

Interesting, @ahoppen should we keep Diagnostic in SwiftSyntax module? It has more general uses than just for parsing.

[getogrand]

This should be merged ASAP, because the main branch cannot even be build currently with below compilation error messages.

• Cannot find type 'DiagnosticEngine' in scope
• Cannot find type 'Diagnostic' in scope

(Tested on 2021.10.03 07:55 GMT-4, M1 Macbook Pro)

[ahoppen]

Interesting, @ahoppen should we keep Diagnostic in SwiftSyntax module? It has more general uses than just for parsing.

I think we should not move Diagnostic back to the SwiftSyntax module. SwiftSyntax should just represent the syntax tree (+ operations on the tree). Since it doesn’t support parsing, there’s no need for diagnostics handling in the SwiftSyntax module. I would be open to discussions moving Diagnostics to a new module that’s neither SwiftSyntax nor SwiftSyntaxParser though, but I’m not sure if that’s gaining us anything.

[akyrtzi]

Since it doesn’t support parsing, there’s no need for diagnostics handling in the SwiftSyntax module

But swift-format is using it as a general diagnostic mechanism for its own diagnostics; within this context it's not just about parser diagnostics.
For example one could create a tool that forms a tree via the APIs, based on some non-swift inputs, and still want to emit diagnostics.

Or maybe you suggest that Diagnostic should only be used for the parser diagnostics and not for anything else?

[allevato]

SwiftSyntax should just represent the syntax tree (+ operations on the tree). Since it doesn’t support parsing, there’s no need for diagnostics handling in the SwiftSyntax module.

At first I felt that SwiftSyntax was the more appropriate place for the diagnostics functionality, but I agree with this as well; it doesn't necessarily feel right to put it there just because other code that uses the functionality in that module might also want to use diagnostics.

I would be open to discussions moving Diagnostics to a new module that’s neither SwiftSyntax nor SwiftSyntaxParser though, but I’m not sure if that’s gaining us anything.

I think there would be value to this but it doesn't specifically help SwiftSyntax in isolation, and that SwiftSyntax probably isn't the right place for it. For example:

Or maybe you suggest that Diagnostic should only be used for the parser diagnostics and not for anything else?

If I adopted this approach, I'd be creating a third (at least?) diagnostics engine among Swift projects—that new one, plus the one in SwiftSyntax, plus the one in tools-support-core that's used by swift-driver and SPM. That makes me think this is useful functionality that should be shared by all three, somewhere. But it's also important to keep SwiftSyntax and SwiftSyntaxParser free of heavy dependencies.

So... here's a bolder idea: maybe the whole concept of a diagnostic engine/consumers shouldn't live in SwiftSyntax/SwiftSyntaxParser at all? If the parser is the only user of it, it could just take a callback function instead of a diagnostic engine reference, and pass its own lightweight Diagnostic value containing information relevant to a parse issue. Then, clients of SwiftSyntaxParser would translate those into whatever diagnostic engine they wanted to use (for example, swift-format could start using TSC instead).

[akyrtzi]

So... here's a bolder idea: maybe the whole concept of a diagnostic engine/consumers shouldn't live in SwiftSyntax/SwiftSyntaxParser at all?

Seems like a good idea that has better separation of concerns 👍

[ahoppen]

Filed https://bugs.swift.org/browse/SR-15280 to keep track of moving DiagnosticEngine to another module.

I would suggest to merge this PR to unblock swift-format until that change has been made.

[akyrtzi]

I'm still concerned with the amount of changes in this PR that would most likely have to be reverted after changes for SR-15280; churn that other clients may also have to deal with and revert, could we move the type in SwiftSyntax module for now?

[allevato]

Coming back around to this, I'm going to close this in favor of #270. Instead of staying coupled to the SwiftSyntaxParser.Diagnostic type (or any other diagnostic engine's type), that PR creates a swift-format-specific type to communicate those back through the API layer, and then limits the use of a specific diagnostic engine (in this case, the one from swift-tools-support-core) to the frontend executable.

This sets us up to make some nice improvements later, like:

1. Splitting out the lint/format methods that parse source code in the SwiftFormat API module into their own module, which would make the core API completely decoupled from SwiftSyntaxParser. This would be useful for code-generation use cases, where someone can build up a tree in-memory and then format it, without needing to link to the parser library.
2. Applying different severity levels to diagnostics based on their category/rule, which was discussed a bit in Add strict flag #252.