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

file_header should ignore doc comments #1719

Closed
mekarthedev opened this issue Jul 27, 2017 · 11 comments
Closed

file_header should ignore doc comments #1719

mekarthedev opened this issue Jul 27, 2017 · 11 comments
Labels
enhancement Ideas for improvements of existing features and rules.

Comments

@mekarthedev
Copy link

mekarthedev commented Jul 27, 2017

My file_header config is:

file_header:
  forbidden_pattern: ".?"

I.e. forbid file headers. But I have some types with documentation comment on them. E.g.

/// This is great tool.
class GreatTool {}

By coincidence the doc comment is the first non-blanc text in the file. As a result, I think, the file_header rule interprets the doc comment as file header and emits a warning.

Ideally, file_header should ignore triple-slash comments prepending some declaration.

@marcelofabri marcelofabri added the enhancement Ideas for improvements of existing features and rules. label Jul 27, 2017
@marcelofabri
Copy link
Collaborator

This is a good idea! Unfortunately, since Swift 2.3 we no longer get the doc comment ranges for declarations.

I'd suggest you to try to solve it with a different pattern instead. ^[^\/]{3} might work in your case.

@SDGGiesbrecht
Copy link
Contributor

SDGGiesbrecht commented Jul 27, 2017

I’d suggest you to try to solve it with a different pattern instead.

That is a viable workaround, but this is a global bug, not something specific to that project.

Why not have the header rule check if the first non‐whitespace, non‐shebang characters in the file are /// or /** (i.e. there is documentation, not a header), and if so, run the forbidden pattern on "" instead?

That or something like it would fix things for all users.

@marcelofabri
Copy link
Collaborator

Because that not necessarily means they're documentation. For example:

/// I prefer to use triple dashes for some reason
/// but this is definitely a header.

import Foundation

@marcelofabri
Copy link
Collaborator

Ideally, we would get documentation locations from SourceKit, get the one for the first declaration and see if it's the first comment in the file or not.

@Uncommon
Copy link
Contributor

If the first comment uses triple slashes, you could go by whether the following statement is a candidate for documentation - import isn't, but class is.

@SDGGiesbrecht
Copy link
Contributor

If the first comment uses triple slashes, you could go by whether the following statement is a candidate for documentation - import isn't, but class is.

I imagine anyone actually using three slashes for headers already has problems with Swift, Xcode’s Quick Help, Jazzy, and every other tool under the sun (correctly) incorporating some of their headers into documentation, whether they have noticed it yet or not.

Is the three‐slash header real‐world usage somewhere, or just hypothetical?

If it were up to me, a three‐slash comment not attached to anything would be a violation in its own right and worthy of its own rule: stray_documentation.

@mekarthedev
Copy link
Author

@marcelofabri, thanks for the workaround. forbidden_pattern: "^//[^/]|/\\*[^*]" worked for me. Going to stay with it for now.

Adding checks for next declaration to file_header impl doesn't seem like a bad idea for now. It may wouldn't be 100% accurate, but at least it doesn't seem to be error prone or non future proof.

@marcelofabri
Copy link
Collaborator

@mekarthedev Glad it worked! I've opened swiftlang/swift#11264 to make SourceKit also return the location of doc comments. If that is accepted, it'd make this enhancement much easier and more reliable.

@jpsim
Copy link
Collaborator

jpsim commented Jul 31, 2017

We don't need that to detect where doc comments are. That's already exposed in the syntax map, see SyntaxKind.docComment and SyntaxKind.docCommentField: https://github.com/jpsim/SourceKitten/blob/0.18.0/Source/SourceKittenFramework/SyntaxKind.swift#L28-L31

@jpsim
Copy link
Collaborator

jpsim commented Jul 31, 2017

That being said, @marcelofabri swiftlang/swift#11264 is awesome and will definitely help in cases where we need to know which doc comments are attached to a given declaration, which will allow us to rebuild the "missing docs" and "invalid docs" rules.

@marcelofabri
Copy link
Collaborator

Totally forgot that they have different syntax kinds. Opened #1730.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules.
Projects
None yet
Development

No branches or pull requests

5 participants