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

valid_docs rule shouldn't be enforced for common UIKit method overrides #284

Closed
litso opened this issue Dec 21, 2015 · 12 comments
Closed

valid_docs rule shouldn't be enforced for common UIKit method overrides #284

litso opened this issue Dec 21, 2015 · 12 comments
Labels
question Question or doubts that needs discussion and clarification. Can become a bug or proposal.

Comments

@litso
Copy link

litso commented Dec 21, 2015

For example if I override shouldAutoroate I shouldn't have to document the return type.

        override func shouldAutorotate() -> Bool {
            return false
        }
@litso litso changed the title valid_docs rule shouldn't be be enforced for common overrides valid_docs rule shouldn't be be enforced for common UIKit method overrides Dec 21, 2015
@litso litso changed the title valid_docs rule shouldn't be be enforced for common UIKit method overrides valid_docs rule shouldn't be enforced for common UIKit method overrides Dec 21, 2015
@keith
Copy link
Collaborator

keith commented Dec 21, 2015

I brought this up somewhere but IMO we should ignore everything that is an override

@proth
Copy link

proth commented Dec 21, 2015

I think so too

@jpsim jpsim added the enhancement Ideas for improvements of existing features and rules. label Dec 24, 2015
@jpsim
Copy link
Collaborator

jpsim commented Dec 25, 2015

The missing_docs rule actually skips all overridden declarations, but the valid_docs rule validates all documentation-style comments (/// and /** */).

So what's the motivating factor for writing documentation-style comments for an overridden declaration without those docs being fully valid? If it's a note on the implementation to developers (e.g. not users of the API), then why not use a regular style comment? If it is for users of the API, the docs should be complete, no?

@jpsim jpsim added question Question or doubts that needs discussion and clarification. Can become a bug or proposal. and removed enhancement Ideas for improvements of existing features and rules. labels Dec 25, 2015
@Sega-Zero
Copy link
Contributor

I have slightly different case with the same warning. Having this protocol and implementation:

/// test proto description
public protocol TestProto {
    /// - Returns: some string
    /// - Parameter sampleParam: yet another parameter
    func sampleFunc(sampleParam: String) -> String
}

/// Sample proto documentation
public struct SampleStruct: TestProto {
    /// - seealso: `TestProto`
    public func sampleFunc(sampleParam: String) -> String {
        return ""
    }
}

I use jazzy for doc generation, SampleStruct is the part of my public API and should be documented. Why do I get a warning on SampleStruct.sampleFunc(_:) implementation?

@jpsim
Copy link
Collaborator

jpsim commented Dec 25, 2015

Why do I get a warning on SampleStruct.sampleFunc(_:) implementation?

Because it is only partially documented.

@Sega-Zero
Copy link
Contributor

Ok, what should be added to make it go away?
experiments swift 2015-12-25 18-19-17

@Sega-Zero
Copy link
Contributor

I don't mind to duplicate the description from the protocol, but that's as weird as documenting the method override, i think.

@norio-nomura
Copy link
Collaborator

Ok, what should be added to make it go away?

@Sega-Zero Current valid_docs rules implementation is case sensitive and expecting lower case keywords such as parameter or returns.
It seems that Xcode ignores case of them, so we need to fixing SwiftLint.

@pcantrell
Copy link

Note that Swiftlint insisting on parameter and returns sections to match the method signature is contrary to Apple’s API design guidelines:

screen shot 2015-12-28 at 10 39 33 am

(This is under “Make documentation comments tool-friendly.”)

@jpsim
Copy link
Collaborator

jpsim commented Dec 29, 2015

Thanks for sharing that link @pcantrell. I've been struggling with this since originally writing the rule, where it's not always appropriate to follow the Swift documentation syntax exactly. Of course, it's always possible to just disable a rule, either locally or globally, but I'm open to loosening the constraints on the valid docs rule to reduce the number of false violations.

@pcantrell
Copy link

It’s the inherent problem of linting tools: the rules that are truly absolute tend to be fairly mindless. Most fall into a nuanced area that requires some human judgement, and deciding where to draw the linter line is a tricky call!

@marcelofabri
Copy link
Collaborator

Closing this since we removed this rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question or doubts that needs discussion and clarification. Can become a bug or proposal.
Projects
None yet
Development

No branches or pull requests

8 participants