java.interop icon indicating copy to clipboard operation
java.interop copied to clipboard

Javadoc "optional" links are not valid

Open jonpryor opened this issue 3 years ago • 3 comments

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.

jonpryor avatar Jun 09 '22 21:06 jonpryor

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

pjcollins avatar Jun 21 '22 18:06 pjcollins

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 (7574f166008bb45c0df97315aae7907ac25f8602)?

    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…)

jonpryor avatar Jun 27 '22 19:06 jonpryor

  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 (7574f166008bb45c0df97315aae7907ac25f8602)?

This isn't entirely accurate, as processing of inline <a href> elements was only ~recently (and only partially) fixed in https://github.com/xamarin/java.interop/commit/13def0e105f876678256a53b2f362380cd4818a0#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 https://github.com/xamarin/java.interop/issues/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 https://github.com/xamarin/java.interop/issues/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>

pjcollins avatar Jun 30 '22 20:06 pjcollins