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

Use inherited docs when local ones are missing #190

Open
jpsim opened this issue Mar 23, 2015 · 33 comments
Open

Use inherited docs when local ones are missing #190

jpsim opened this issue Mar 23, 2015 · 33 comments

Comments

@jpsim
Copy link
Collaborator

jpsim commented Mar 23, 2015

For example, Alamofire documents declarations in its protocols, but not the implementations of the types inheriting from those protocols.

See URLStringConvertible as an example.

@petrmanek
Copy link

Any progress on this? It's been over a year.

@jpsim
Copy link
Collaborator Author

jpsim commented May 1, 2016

No progress on this to report, the task is up for the taking. Why not take this opportunity to contribute to a feature you need, @petrmanek?

@pigeondotdev
Copy link
Contributor

Any progress on this? @petrmanek @jpsim

@petrmanek
Copy link

@istx25 None at all. I'd be happy to contribute, but I'm not a ruby guy...

@pigeondotdev
Copy link
Contributor

Neither am I! A guy or experienced with Ruby. hahahah. 😉 I too am basically/completely new to Ruby and I got started as of two days ago. Use this as an opportunity to learn and write Ruby. There are plenty of people that will be able to provide feedback and help you in the progress.

@pigeondotdev
Copy link
Contributor

Never let not being experienced with something stop you. 💕 Heck, we could implement the feature together.

@jpsim
Copy link
Collaborator Author

jpsim commented Nov 26, 2016

No Ruby experience necessary. This feature could be written entirely in Swift, since it falls squarely in the purview of SourceKitten.

@pigeondotdev
Copy link
Contributor

the purr view of SourceKitten. 😆

@pigeondotdev
Copy link
Contributor

But sounds good to me. Even better. I have Swift experience so I'm even more comfortable implementing this at some point if no one else grabs it.

@reitzig
Copy link

reitzig commented Apr 24, 2017

Definitely! This is basically expected if you come over from the Java world.

@reitzig
Copy link

reitzig commented May 19, 2017

Thought:

/// Makes objects of type `T`
protocol Maker {
	associatedtype T
	/// Makes a `T`
	static func make(from: String) -> T
}

class Foo: Maker {
	typealias T = Int
	static func make(from: String) -> Int {
		return Int(from)
	}
}

When inheriting the documentation from the protocol, should T be replaced with Int? I should think so.

@ronak2121
Copy link

Any progress on this? We just ran into this...

@reitzig
Copy link

reitzig commented Mar 7, 2018

Related: If A: B and A inherits foo() from B, A.foo() does not auto-link.

@galli-leo
Copy link

Interestingly enough, Xcode automatically adds the comments when displaying the "interface" of a source file (by using the side by side view):

// Test.swift
class Test: CustomStringConvertible {
    public var description: String {
        return "Ttest"
    }
}
// Test.swift (interface)
internal class Test : CustomStringConvertible {

    /// A textual representation of this instance.
    ///
    /// Calling this property directly is discouraged. Instead, convert an
    /// instance of any type to a string by using the `String(describing:)`
    /// initializer. This initializer works with any type, and uses the custom
    /// `description` property for types that conform to
    /// `CustomStringConvertible`:
    ///
    ///     struct Point: CustomStringConvertible {
    ///         let x: Int, y: Int
    ///
    ///         var description: String {
    ///             return "(\(x), \(y))"
    ///         }
    ///     }
    ///
    ///     let p = Point(x: 21, y: 30)
    ///     let s = String(describing: p)
    ///     print(s)
    ///     // Prints "(21, 30)"
    ///
    /// The conversion of `p` to a string in the assignment to `s` uses the
    /// `Point` type's `description` property.
    public var description: String { get }
}

Maybe this can be used somehow?

@SDGGiesbrecht
Copy link
Contributor

At some point I noticed Xcode had started doing a better job of this, but until now I never stopped to investigate exactly what it does.

At this point (Xcode 9.4), Quick Help does this perfectly. Protocol conformance methods fall back to the protocol definitions, and subclass overrides fall back to their parent methods. Subclassed methods even get a note about how far up the inheritance chain the documentation came from.

I think I would prefer if these were exempt from the coverage percentage and warnings, because from now on I actually want them blank so that they can inherit.

As for the rendered documentation, the simplest would be to consider them implicit and not list them in the documentation. How many entries for == do we really want to see? I think that is what I would prefer, but I would not object to having them repeated with the inherited documentation.

@galli-leo
Copy link

galli-leo commented Jun 3, 2018

@SDGGiesbrecht I disagree with hiding them, since then someone reading the documentation cannot be sure they are implemented. But this should definitely be a configurable option.

@jpsim Regarding the implementation, not sure what the best way to go about this is. One could be to open an issue with Swift itself. Another option would be to first generate the interface file for everything and then run documentation on that. The third option would be to use the method described in the linked issue to a) extract the protocol / superclass info and lookup documentation for that. Though this is more of a sourcekitten issue.

@SDGGiesbrecht
Copy link
Contributor

I disagree with hiding them, since then someone reading the documentation cannot be sure they are implemented.

I don’t understand. If the source even compiled, then the methods must be available. If a type conforms to Equatable, then == is available. Period. It is irrelevant to a client developer whether it was directly implemented, inherited from a superclass, received from a default implementation, or synthesized by the compiler. All that matters is that SomeType: Equatable or the like appears in the documentation. Anything more is redundant. (Unless the override does something unusual, but then it should be documented separately and not left to inheritance.)

I can also see the use of having a complete list of everything the type can do, including all inherited capabilities. I realize a client may just want to know what a type can do, not sort through an unfamiliar conformance tree. I would definitely vouch for that style too, especially with an “Expand Conformances” button so the reader can toggle at will. The difficulty is that it would require a lot more that just displaying inherited documentation for whatever overrides are found. Take Equatable for example. Every conforming type would have == in its list, but none of them would have !=. For more elaborate protocols like Collection, the number of such “hidden” methods balloons very quickly. So instead of giving readers a piecemeal list, I would rather train them to investigate the conformance hierarchy where most of the information really is.

@johnfairh
Copy link
Collaborator

Hi @galli-leo, it would be great if you're interested in working on this. The way I think we should do this is to make use of the inherited docs that Jazzy already has (from SourceKit request.cursorinfo) -- these are in the key.doc.full_as_xml field inside the CommentParts element and what is used to generate the Xcode quick help.

The jazzy code needed is to process this XML into HTML -- not a huge task but more than a few minutes. Can offer some more pointers if you are interested! This would work out of the box on Linux as well.

@reitzig
Copy link

reitzig commented Jun 4, 2018

As for the rendered documentation, the simplest would be to consider them implicit and not list them in the documentation. How many entries for == do we really want to see? I think that is what I would prefer, but I would not object to having them repeated with the inherited documentation.

The documentation may have been written in terms of associated types and/or type parameters which are mode concrete by the implementing type. It might be useful to repeat both signature and documentation with the concrete types.

That said, having a separate section for inherited methods linking to their implementation/documentation might be prudent. Another idea is a link to where the implementation comes from.

@galli-leo
Copy link

@SDGGiesbrecht

I don’t understand. If the source even compiled, then the methods must be available. If a type conforms to Equatable, then == is available. Period.

I meant more in reference to custom protocols, where a user might not have a quick way to check if a certain protocol is supported. Additionally, the point of documentation is not to have the compiler run something to find out if a method exists.

I do think for simple protocols (e.g. Equatable) it makes sense to hide them.

I can also see the use of having a complete list of everything the type can do, including all inherited capabilities. I realize a client may just want to know what a type can do, not sort through an unfamiliar conformance tree. I would definitely vouch for that style too, especially with an “Expand Conformances” button so the reader can toggle at will. The difficulty is that it would require a lot more that just displaying inherited documentation for whatever overrides are found. Take Equatable for example. Every conforming type would have == in its list, but none of them would have !=. For more elaborate protocols like Collection, the number of such “hidden” methods balloons very quickly. So instead of giving readers a piecemeal list, I would rather train them to investigate the conformance hierarchy where most of the information really is.

My idea would be to have a section inside the declaration, that can be opened up to reveal all classes or protocols a function is inherited from, with their respective doc comment and a link to them. Very basic hacked together example:

inherited_documentation

@johnfairh My idea would unfortunately not work when using the key.doc.full_as_xml, as that won't give us a reference to what type a function was inherited from. Additionally, I would like to implement linking std entries (e.g. String reference or CustomStringConvertible as seen above) as well.

@reitzig

That said, having a separate section for inherited methods linking to their implementation/documentation might be prudent. Another idea is a link to where the implementation comes from.

Yeah I think that's the best option, especially since you can inherit a function from multiple classes and or protocol. (e.g. overriding a method in a subclass that also is required by a protocol). In such cases it makes sense (imo) to have an overview of both the superclass and protocol documentation.

@johnfairh
Copy link
Collaborator

Well, key.overrides is the list of symbols being overridden. For symbols not in the current module (eg. stdlib) that would need a further request.cursorinfo querying by USR if you wanted the type definition. Anyway - look forward to seeing what you come up with.

@galli-leo
Copy link

@johnfairh Ah thanks for the pointers, maybe my issue with SourceKitten isn't even needed in that case. Though I guess the only way to get the correct documentation location for stdlib, would still be the custom xcode requests? (I don't think they are in the SourceKitten issue (jpsim/SourceKitten#525), but there is a request that get's you the location of the docset and the location of a stdlib (or iOS framework) symbol inside the docset)

@johnfairh
Copy link
Collaborator

the location of the docset and the location of a stdlib (or iOS framework) symbol inside the docset

Oh exciting! Be nice if we could use that output to construct a portable link to the online docs (see veeeeery old issue #13) - but either way these APIs are definitely worth investigating.

@galli-leo
Copy link

@johnfairh Unfortunately that command was removed in Xcode 9.3, that's why I want to go this route (The output of the aforementioned request looks quite similar though, so I guess they just moved it to SourceKit). Will try to get a sample of that request today or tomorrow.

@galli-leo
Copy link

So I must have misremembered, but unfortunately, the request I had in mind, does not give us any information regarding the documentation location.

However, after some digging and reverse engineering the Framework Xcode uses for Documentation, I found the following file: Xcode.app/Contents/SharedFrameworks/DNTDocumentationSupport.framework/Versions/A/Resources/external/map.db`. This has a list of the links to all the external documentations, like this:

row_id	uuid		topic_id		source_language	reference_path
"18737"	"hsAewr81cJ"	"1614625"	"0"	"iad/adbannerviewdelegate/1614625-bannerviewdidloadad"
"45335"	"hsmY90yIeE"	"1434476"	"0"	"appkit/nsviewcontroller/1434476-viewdidload"
"45396"	"hszdX_5qrG"	"1388874"	"0"	"appkit/nssplitviewcontroller/1388874-viewdidload"
"45444"	"hsMk0PaeJf"	"1428253"	"0"	"appkit/nstabviewcontroller/1428253-viewdidload"
"67461"	"hcAewr81cJ"	"1614625"	"1"	"iad/adbannerviewdelegate/1614625-bannerviewdidloadad"
"111359"	"hsbgq4DSVm"	"1621495"	"0"	"uikit/uiviewcontroller/1621495-viewdidload"
"165086"	"hcmY90yIeE"	"1434476"	"1"	"appkit/nsviewcontroller/1434476-viewdidload"
"165145"	"hczdX_5qrG"	"1388874"	"1"	"appkit/nssplitviewcontroller/1388874-viewdidload"
"165226"	"hcMk0PaeJf"	"1428253"	"1"	"appkit/nstabviewcontroller/1428253-viewdidload"
"172696"	"hcbgq4DSVm"	"1621495"	"1"	"uikit/uiviewcontroller/1621495-viewdidload"

reference_path is used to append to the documentation url (e.g. https://developer.apple.com/documentation/uikit/uiviewcontroller/1621495-viewdidload) and source_language is for what language is used (0 swift, 1 objective-c and 2 js). Unfortunately, I haven't been able to decipher how the uuid is generated, so the file is not yet that useful.

I already found out that the first to letters have some meaning (first letter is either h, t or c and the second letter is s for swift, c for objective c and j for javascript). Maybe the other part is a hash of somesorts? If you have any idea what it might be or how Xcode is exactly reading from this db with a symbol, please let me know.

@SDGGiesbrecht
Copy link
Contributor

I have been very happy to see the ball rolling on this again, and I think all of the discussed possibilities are an improvement from the status quo, so I say this somewhat regrettably...

It is no longer clear to me whether we are still talking about re‐using inherited content, or just providing a link to it published elsewhere. Unfortunately it occurred to me today that if we are actually using the content, then we run into copyright and licensing issues [exasperated sigh]. Just because Xcode has access to a .swiftdoc file does not mean the corresponding module is open source.

Even the documentation of the Standard Library asserts copyright and declares “All rights reserved.” (In this case though—since whatever can be parsed from the raw source is under the same licence as the source itself—a complete re‐use of Standard Library documentation comments would be fine within the terms of its Apache licence.)

But other modules, such as AppKit, have almost no source available for use under any licence. Forwarding a Quick Help note inherited from AppKit into documentation generated by Jazzy would be a violation of international copyright law—especially if the result were then published online.

That fact throws annoying complications into what we are considering here:

A. Links to external things are fine.
B. Use of content from the same module is fine.
C. Use of content from another module is fine as long as the author is the same or there is a compatible licence (such as most open source).
D. Use of any additional content risks a legal suit.

A and B are safe to do by default. C would probably have to be opt‐in on a per‐module basis, since there is no way Jazzy can tell which modules belong to C and which belong to D.

@galli-leo
Copy link

galli-leo commented Jul 10, 2018

@SDGGiesbrecht
You raise a good point, but I think we really don‘t have to worry, due to a few reasons.

  1. First and foremost, since this will be a toggable option, we can just add a legal disclaimer when using it. Then it‘s the users problem (Though they won‘t have one as explained below)

  2. The whole thing would be protected under fair use. What we would actually be doing, is not copying the whole source code including comments, that would illegal. What we actually do, is provide a little summary for something apple has made and then link to it. It is like quoting The Verge in an article and then linking to them. Additionally, we provide the whole thing just as a convenience. The whole documentation is comprised of more than just snippets of apple documentation.

  3. If we stylize the whole thing as a quote, it wouldn’t even be copyright infringement, even if we only had other doc comments.

  4. They don‘t care. I don‘t think anybody at apple cares whether their documentation comments are found somewhere else with a link.

  5. Xcode does it as well. While Xcode obviously has the rights to do so for their frameworks, they don‘t have them for any others. When you generate an interface file, they still copy over the documentation comment. If apple lawyers thought that was ok, then we will be fine as well.

  6. IMO the most important question in such cases, is whether apple will ever sue. The chances for that are almost 0, since open source projects aren‘t typically known for their stash of cash :P. Additionally, they probably couldn‘t even sue for something, since they already provide all comments free of charge and no one makes money off of documentation.

  7. Some documentation comments aren‘t even protected by copyright. If you just have a short comment, like description provides a Description of an object, it‘s not „creative“ enough to even have copyright protection

All in all, just make it an extra option with a small legal disclaimer and jazzy doesn‘t have any issue. Something like „Enabling this option could potentially open yourselfs to lawsuits. While quoting external documentation should fall under fair use, companies could still try to sue.“

I strongly think that this feature would never cause any copyright issues, unless you somehow only have inherited docs and don‘t change any of them. Still you could then argue that the specific arrangement and styling of the „quotes“ is fair use.

P.S.

Even the documentation of the Standard Library asserts copyright and declares “All rights reserved.”

That‘s for the whole developer page, isn‘t it? Also it‘s a standard thing to put at the bottom of any page, doesn‘t mean anything. Copyright is granted to everything, you don‘t have to explicitly declare it.

@SDGGiesbrecht
Copy link
Contributor

@galli-leo,

First and foremost, since this will be a toggable option, we can just add a legal disclaimer when using it. Then it‘s the users problem.

That’s good enough if its the best that can be done. But it would still be nice to turn it on and off for individual modules if it is not too hard to implement that way.

The whole thing would be protected under fair use.

The Doctrine of Fair Use only carries legal weight in the United States. (Though some countries do have roughly comparable rules—but not all.) Jazzy may be published from the US, but its users are everywhere, and it is the user doing the copying.

What we actually do, is provide a little summary for something apple has made and then link to it. It is like quoting The Verge in an article and then linking to them. [...] If we stylize the whole thing as a quote, it wouldn’t even be copyright infringement.

The right to quote is the second principle of the Berne Convention, signed by the entire industrialized world. The problem is that it is subject to a “reasonable limit” (interpreted differently by different countries). If someone publishes a module with a SortedArray, inheriting 10+ methods from Collection and adding only a comment for the structure itself, they would be exceeding their right to quote in many jurisdictions.

Xcode does it as well.

Xcode only displays what it is given (either source or a .swiftdoc file). It makes no long‐term copies and publishes nothing. A compiled library could have been published and the binary licensed only for use as‐is, not for modification. Xcode can still display the documentation (just like a Blue‐Ray player is allowed to pipe an image to your screen), but Jazzy would be making a (modified) copy in violation of the licence (just like you cannot make a back‐up of the Blue‐Ray disc, depending on the country).

IMO the most important question in such cases, is whether apple will ever sue. The chances for that are almost 0.

I highly doubt that as well. But in some countries it is the state, not the copyright holder, who is responsible to bring charges. (The Agreement on Trade-Related Aspects of Intellectual Property Rights even requires all states to do so for infringement on larger scales, but Jazzy’s output would never reach that scale.) There are also third‐party modules to consider, not just Apple’s. Even then I agree it is unlikely, but it comes down to a moral question: “Is it okay to speed just because there are no police around?” (You have a right to remain silent. :P)

Since open source projects aren‘t typically known for their stash of cash :P

Jazzy’s users are not limited to open source projects.

All in all, just make it an extra option with a small legal disclaimer and jazzy doesn‘t have any issue.

Jazzy (i.e. Realm) would never be the one liable (or else such a country’s laws need fixing). It would be the user who is liable.

Something like „Enabling this option could potentially open yourselfs to lawsuits. While quoting external documentation should fall under fair use, companies could still try to sue.“

Something like that, yes. But maybe less frightening: “Be aware that this will copy documentation from dependencies. Make sure you have the rights to do so.” Again, it would be nice to toggle for specific modules.

@J7mbo
Copy link

J7mbo commented Aug 27, 2018

Just thought I'd pop in and say I'd love this feature, similar to how modern PHP has it with phpdoc. The docs in PHP look like this:

/**
 * @inheritdoc
 */

I'd love to see this with swiftdoc and Jazzy supporting it:

/// inheritdoc

or similar...

@SDGGiesbrecht
Copy link
Contributor

@J7mbo,

I’d love to see this with swiftdoc and Jazzy supporting it:

/// inheritdoc

or similar...

Manually triggered inheritance like this can be done with Workspace before running Jazzy, at least when the parent source code is available.

This issue thread is more about making the whole process completely automatic like it is in Xcode.

@groue
Copy link

groue commented Jul 5, 2020

Hello, why is this issue tagged as "priority-low"? This is a major feature.

@markuswinkler
Copy link

Any news on this?

@strfn
Copy link

strfn commented Nov 3, 2021

Plus one on this one.

Any best practice people are applying to overcome this limitation while not implemented ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests