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

Javadoc "optional" links are not valid #993

Open
jonpryor opened this issue Jun 9, 2022 · 3 comments
Open

Javadoc "optional" links are not valid #993

jonpryor opened this issue Jun 9, 2022 · 3 comments
Assignees
Labels
bug Component does not function as intended javadoc Issues with consuming Java documentation formats

Comments

@jonpryor
Copy link
Member

jonpryor commented Jun 9, 2022

Consider this documentation page: https://docs.microsoft.com/en-us/dotnet/api/java.util.concurrent.iblockingdeque.removelastoccurrence?view=xamarin-android-sdk-12

Which contains:

Exceptions
ClassCastException
if the class of the specified element is incompatible with this deque (optional)

The "optional" link refers to an HTTP-404: http://developer.android.com/Collection.html#optional-restrictions

We should figure out where this link is coming from and fix it.

@pjcollins
Copy link
Member

The corresponding Android API docs also point to a dead link, though it is different:

https://java.base/java/util/Collection.html#optional-restrictions

We could try to manually fix this up to point to https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Collection.html#optional-restrictions which seems valid

@jonpryor
Copy link
Member Author

OK, so there are two issues here. Background: the relevant Javadoc contains a relative URL:

public /* partial */ interface BlockingDeque<E> extends BlockingQueue<E>, Deque<E> {
    /**
     * …
     * @throws ClassCastException if the class of the specified element
     *         is incompatible with this deque
     * (<a href="../Collection.html#optional-restrictions">optional</a>)
     * @throws NullPointerException if the specified element is null
     * (<a href="../Collection.html#optional-restrictions">optional</a>)
     */
    boolean removeLastOccurrence(Object o);
}

Note the <a href="../Collection.html#optional-restrictions">.

  1. We're replacing/providing the domain for all URLs, which is presumably why "no domain" becomes http://developer.android.com. Is it correct to always use the //javadoc-metadata/link/@prefix value (7574f16)?

    What happens if the Javadoc contains <a href="http://example.com">example</a>? Do we preserve the domain at all?

    We may need to reconsider when the //javadoc-metadata/link/@prefix value is used.

  2. Google's tooling is generating an invalid URL for their docs. (They start with ../Collection.html, and wind up with java.base/java/util/Collection.html. This makes some sense, in that Collection is defined in the java.base.jmod Java module {JDK 11+}, but Android doesn't support Java modules, so this is still very odd.)

These issues are orthogonal; (2) not happening would not necessarily address (1). Fixing (1) -- whatever that means -- would not address (2).

Which brings us to @pjcollins suggestion:

We could try to manually fix this up

What is the manner for the fixup? Via tooling? (Via which file formats/parameters/etc.?). Human manually? (When?)

I'm interested in re-considering Issue (1), but I'm not at all sure that we can "reasonably" fix these "invalid 'optional' links". (Fortunately there are only like 5 of them, so ignoring them wouldn't be a major setback…)

@pjcollins
Copy link
Member

  1. We're replacing/providing the domain for all URLs, which is presumably why "no domain" becomes http://developer.android.com. Is it correct to always use the //javadoc-metadata/link/@prefix value (7574f16)?

This isn't entirely accurate, as processing of inline <a href> elements was only ~recently (and only partially) fixed in 13def0e#diff-cecf6fac1d261bc8e4349871be05237baa930f2613605cf0fee8183f6dc4fd50R98-R127. I say it was only partially fixed because we don't attempt to construct a full URL for any relative links, and there are instances where attempting to parse these elements still fails.

We are also currently ignoring @throws elements entirely when generating C# doc comments, as mentioned in #843. These broken links are left over from the initial documentation import 9 years ago. Our C# docs for this member currently look like this:

/// <param name="o">element to be removed from this deque, if present</param>
/// <summary>Removes the first occurrence of the specified element from this deque.</summary>
/// <remarks>
///   <para>
///     <format type="text/html">
///       <a href="https://developer.android.com/reference/java/util/concurrent/BlockingDeque#removeFirstOccurrence(java.lang.Object)" title="Reference documentation">Java documentation for <code>java.util.concurrent.BlockingDeque.removeFirstOccurrence(java.lang.Object)</code>.</a>
///     </format>
///   </para>
///   <para>
///         Portions of this page are modifications based on work created and shared by the 
///         <format type="text/html"><a href="https://developers.google.com/terms/site-policies" title="Android Open Source Project">Android Open Source Project</a></format>
///          and used according to terms described in the 
///         <format type="text/html"><a href="https://creativecommons.org/licenses/by/2.5/" title="Creative Commons 2.5 Attribution License">Creative Commons 2.5 Attribution License.</a></format></para>
/// </remarks>
/// <returns>
///   <c>true</c> if an element was removed as a result of this call</returns>
[Register ("removeFirstOccurrence", "(Ljava/lang/Object;)Z", "GetRemoveFirstOccurrence_Ljava_lang_Object_Handler:Java.Util.Concurrent.IBlockingDequeInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
bool RemoveFirstOccurrence (Java.Lang.Object? o);

When I mentioned a manual fix up I was suggesting a quick human pass to hand edit the few docs that have these broken links. This should be relatively low cost and would persist until #843 is fixed. At that time the tooling would convert the relative link in the Java doc to plain text, something like:

         <exception cref="T:Java.Lang.ClassCastException">if the class of the specified element
          is incompatible with this deque
-         (<format type="text/html"><a href="http://developer.android.com/reference/../Collection.html#optional-restrictions">optional</a></format>)</exception>
+         ("../Collection.html#optional-restrictions"&gt;optional)</exception>

@jpobst jpobst added bug Component does not function as intended javadoc Issues with consuming Java documentation formats labels Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Component does not function as intended javadoc Issues with consuming Java documentation formats
Projects
None yet
Development

No branches or pull requests

3 participants