Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@ final class BuilderUtil {
private static final Pattern XREF_LINK_RESOLVE_PATTERN = Pattern.compile("(?<class>\\w+)\\#(?<member>\\w+)(\\((?<param>.*)\\))?");
public final static String[] LANGS = {"java"};

static String populateUidValues(String text, LookupContext lookupContext) {
/**
* Replaces all the references the link in the linkContent with the UID
*
* @param text Text that potentially contains a link reference
* @param packageName MetaFileItem's packageName
* @param lookupContext Lookup Context
* @return Text with a link reference to the full qualified link
*/
static String populateUidValues(String text, String packageName, LookupContext lookupContext) {
if (StringUtils.isBlank(text)) {
return text;
}
Expand All @@ -51,21 +59,29 @@ static String populateUidValues(String text, LookupContext lookupContext) {
}

String linkContent = linkContentMatcher.group();
String uid = resolveUidFromLinkContent(linkContent, lookupContext);
String uid = resolveUidFromLinkContent(linkContent, packageName, lookupContext);
String updatedLink = linkContentMatcher.replaceAll(uid);
text = StringUtils.replace(text, link, updatedLink);
}
return text;
}

/**
*
* The linkContent could be in following format
* #memeber
* Class#member
* Class#method()
* Class#method(params)
* 1. #member
* 2. Class#member
* 3. Class#method()
* 4. Class#method(params)
* 5. Package.Class# +{Any combination of above}
* All Possibilities listed: https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#link
*
* @param linkContent Text of the link
* @param packageName MetaFileItem's packageName
* @param lookupContext LookupContext
* @return Fully qualified link url
*/
static String resolveUidFromLinkContent(String linkContent, LookupContext lookupContext) {
static String resolveUidFromLinkContent(String linkContent, String packageName, LookupContext lookupContext) {
if (StringUtils.isBlank(linkContent)) {
Comment on lines +84 to 85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's possible not to add the additional parameter and leveraging lookupContext? For me, right now, it seems that the bug comes from lookupContext. I see the variable holds the fully-qualified class name for symbols.

Screen Shot 2022-06-17 at 4 34 17 PM

return "";
}
Expand All @@ -74,19 +90,42 @@ static String resolveUidFromLinkContent(String linkContent, LookupContext lookup

// complete class name for class internal link
if (linkContent.startsWith("#")) {
// Can't use packageName because it is missing the ClassName
String firstKey = lookupContext.getOwnerUid();
linkContent = firstKey + linkContent;
}

// fuzzy resolve, target for items from project external references
String fuzzyResolvedUid = resolveUidFromReference(linkContent, lookupContext);

// exact resolve in lookupContext
linkContent = linkContent.replace("#", ".");
String exactResolveUid = resolveUidByLookup(linkContent, lookupContext);
String qualifiedLink = getFullyQualifiedLinkUrl(linkContent, packageName);

// First, prefer references inside local package
String exactResolveUid = resolveUidByLookup(qualifiedLink, lookupContext);
if (exactResolveUid.isEmpty()) {
// Resolve with original linkContent
exactResolveUid = resolveUidByLookup(linkContent, lookupContext);
}
// Resolve with fuzzyResolve / external references
return exactResolveUid.isEmpty() ? fuzzyResolvedUid : exactResolveUid;
}

/**
* Logic to ensure that resulting link is in the format Package.Class.Method(Params...)
*
* @param linkContent String of the Class#Method
* @param packageName Package Name that should go in front of the link
* @return Fully qualified link url
*/
private static String getFullyQualifiedLinkUrl(String linkContent, String packageName) {
// If packageName does not exist/error'd or is already at the beginning of the link
if (packageName == null || linkContent.indexOf(packageName) == 0) {
return linkContent;
}
return String.format("%s.%s", packageName, linkContent);
}

static List<String> splitUidWithGenericsIntoClassNames(String uid) {
uid = RegExUtils.removeAll(uid, "[>]+$");
return Arrays.asList(StringUtils.split(uid, "<"));
Expand Down Expand Up @@ -120,27 +159,43 @@ else if (uid != "")
return specList;
}

/**
* Populate the links for references based on a UID. Looker generates mappings for local context
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Populate the links for references based on a UID. Looker generates mappings for local context
* Populates the links for references based on a UID. Looker generates mappings for local context

* (file specific) and global context (all the UID references). Searching is done first in the
* local context and then in the global context if local context is not found.
*
* ex. {@link Lookup } would search all the references to find which Lookup file to use
* It would parse the text to search ("Lookup")
*
* Since packages (v1 and v1beta) may contain the same generated java file names, there may be some
* conflicts between which link it should be.
Comment on lines +170 to +171
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add why Javac doesn't have the problem of ambiguity but this tool via javadoc has trouble finding the exact class?

*
* The Looker#consumer() function guarantees that the UID (+ other combinations) will be put into the LookupContext.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is Looker?

* As long as the link that we extract contains Package#Method, it will match in either local or global context
*
* @param packageMetadataFiles Package specific metadata files
* @param classMetadataFiles Class specific metadata files
*/
static void populateUidValues(List<MetadataFile> packageMetadataFiles, List<MetadataFile> classMetadataFiles) {
Lookup lookup = new Lookup(packageMetadataFiles, classMetadataFiles);

classMetadataFiles.forEach(classMetadataFile -> {
LookupContext lookupContext = lookup.buildContext(classMetadataFile);

for (MetadataFileItem item : classMetadataFile.getItems()) {
String packageName = item.getPackageName();
item.setSummary(YamlUtil.cleanupHtml(
populateUidValues(item.getSummary(), lookupContext)
populateUidValues(item.getSummary(), packageName, lookupContext)
));

Optional.ofNullable(item.getSyntax()).ifPresent(syntax -> {
Optional.ofNullable(syntax.getParameters()).ifPresent(
methodParams -> methodParams.forEach(
param -> {
param.setDescription(populateUidValues(param.getDescription(), lookupContext));
})
param -> param.setDescription(populateUidValues(param.getDescription(), packageName, lookupContext)))
);
Optional.ofNullable(syntax.getReturnValue()).ifPresent(returnValue ->
returnValue.setReturnDescription(
populateUidValues(syntax.getReturnValue().getReturnDescription(), lookupContext)
populateUidValues(syntax.getReturnValue().getReturnDescription(), packageName, lookupContext)
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,18 @@

public class Lookup {

private Map<String, String> globalLookup = new HashMap<>();
private Map<String, Map<String, String>> localLookupByFileName = new HashMap<>();
private static final int INITIAL_GLOBAL_CAPACITY = 250000;

private final Map<String, String> globalLookup;
private final Map<String, Map<String, String>> localLookupByFileName;

private final String UID_PACKAGE_NAME_REGEXP = "^.*?\\.(?=[A-Z].*)";
private final String PARAM_PACKAGE_NAME_REGEXP = "(?<=[\\( ]).*?(?=[A-Z].*)";
private final String METHOD_PARAMS_REGEXP = "\\s[^\\s]+?(?=[,)])";

public Lookup(List<MetadataFile> packageMetadataFiles, List<MetadataFile> classMetadataFiles) {
this.globalLookup = new HashMap<>(INITIAL_GLOBAL_CAPACITY);
this.localLookupByFileName = new HashMap<>();
consume(packageMetadataFiles);
consume(classMetadataFiles);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
import java.util.Map;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;

public class BuilderUtilTest {
@Test
public void populateUidValues() {
public void testPopulateUidValues() {
MetadataFile classMetadataFile = new MetadataFile("output", "name");

MetadataFileItem ownerClassItem = buildMetadataFileItem("a.b.OwnerClass", "Not important summary value");
Expand All @@ -41,15 +42,21 @@ public void populateUidValues() {
populateSyntax(item1, "SomeClass#someMethod(String param)");
MetadataFileItem item2 = buildMetadataFileItem("UID known class", "SomeClass#someMethod(String param)");
MetadataFileItem item3 = buildMetadataFileItem("UID method only", "#someMethod2(String p1, String p2)");
item3.setPackageName("a.b");
classMetadataFile.getItems().addAll(Arrays.asList(ownerClassItem, item1, item2, item3));

MetadataFileItem reference1 = new MetadataFileItem("a.b.SomeClass.someMethod(String param)");
reference1.setPackageName("a.b");
reference1.setNameWithType("SomeClass.someMethod(String param)");
MetadataFileItem reference2 = new MetadataFileItem("a.b.OwnerClass.someMethod2(String p1, String p2)");
reference2.setPackageName("a.b");
reference2.setNameWithType("OwnerClass.someMethod2(String p1, String p2)");
classMetadataFile.getReferences().addAll(Arrays.asList(reference1, reference2));
MetadataFileItem reference3 = new MetadataFileItem("c.d.OwnerClass.someMethod2(String p1, String p2)");
reference3.setPackageName("c.d");
reference3.setNameWithType("OwnerClass.someMethod2(String p1, String p2)");
classMetadataFile.getReferences().addAll(Arrays.asList(reference1, reference2, reference3));

BuilderUtil.populateUidValues(Collections.emptyList(), Arrays.asList(classMetadataFile));
BuilderUtil.populateUidValues(Collections.emptyList(), List.of(classMetadataFile));

assertEquals("Wrong summary for unknown class", item1.getSummary(),
"Bla bla <xref uid=\"\" data-throw-if-not-resolved=\"false\">UnknownClass</xref> bla");
Expand Down Expand Up @@ -79,21 +86,33 @@ private void populateSyntax(MetadataFileItem item, String value) {
}

@Test
public void determineUidByLinkContent() {
public void testDetermineUidByLinkContent() {
// Map similar to what is created in Lookup#consume()
Map<String, String> lookup = new HashMap<>() {{
put("SomeClass", "a.b.c.SomeClass");
put("SomeClass.someMethod()", "a.b.c.SomeClass.someMethod()");
put("SomeClass.someMethod(String param)", "a.b.c.SomeClass.someMethod(String param)");
put("a.b.c.SomeClass", "a.b.c.SomeClass");
put("a.b.c.SomeClass.someMethod()", "a.b.c.SomeClass.someMethod()");
put("a.b.c.SomeClass.someMethod(String param)", "a.b.c.SomeClass.someMethod(String param)");
// Duplicate entry to simulate same ClassName in a different package
put("SomeClass", "d.e.f.SomeClass");
put("d.e.f.SomeClass", "d.e.f.SomeClass");
}};

String packageName = "a.b.c";
String otherPackageName = "d.e.f";

LookupContext lookupContext = new LookupContext(lookup, lookup);
assertEquals("Wrong result for class", BuilderUtil.
resolveUidByLookup("SomeClass", lookupContext), "a.b.c.SomeClass");
assertEquals("Wrong result for method", BuilderUtil.
resolveUidFromLinkContent("SomeClass#someMethod()", lookupContext), "a.b.c.SomeClass.someMethod()");
assertEquals("Wrong result for method with param", BuilderUtil.
resolveUidFromLinkContent("SomeClass#someMethod(String param)", lookupContext),
assertEquals("Wrong result for class",
BuilderUtil.resolveUidFromLinkContent("SomeClass", packageName, lookupContext), "a.b.c.SomeClass");
assertEquals("Wrong result for method",
BuilderUtil.resolveUidFromLinkContent("SomeClass#someMethod()", packageName, lookupContext), "a.b.c.SomeClass.someMethod()");
assertEquals("Wrong result for method with param",
BuilderUtil.resolveUidFromLinkContent("SomeClass#someMethod(String param)", packageName, lookupContext),
"a.b.c.SomeClass.someMethod(String param)");
assertEquals("Wrong result for class with duplicate className",
BuilderUtil.resolveUidFromLinkContent("SomeClass", otherPackageName, lookupContext), "d.e.f.SomeClass");

assertEquals("Wrong result for unknown class", BuilderUtil.
resolveUidByLookup("UnknownClass", lookupContext), "");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ items:
fullName: "com.microsoft.samples.google.SpeechSettings"
type: "Class"
package: "com.microsoft.samples.google"
summary: "Settings class to configure an instance of <xref uid=\"com.microsoft.samples.google.v1p1alpha.SpeechClient\" data-throw-if-not-resolved=\"false\">SpeechClient</xref>.\n\n <p>The default instance has everything set to sensible defaults:\n\n <ul>\n <li>The default service address (speech.googleapis.com) and default port (443) are used.\n <li>Credentials are acquired automatically through Application Default Credentials.\n <li>Retries are configured for idempotent methods but not for non-idempotent methods.\n </ul>\n\n <p>The builder of this class is recursive, so contained classes are themselves builders. When\n build() is called, the tree of builders is called to create the complete settings object.\n\n <p>For example, to set the total timeout of recognize to 30 seconds:\n\n <pre class=\"prettyprint lang-java\"><code>\n SpeechSettings.Builder speechSettingsBuilder = SpeechSettings.newBuilder();\n speechSettingsBuilder\n .recognizeSettings()\n .setRetrySettings(\n speechSettingsBuilder\n .recognizeSettings()\n .getRetrySettings()\n .toBuilder()\n .setTotalTimeout(Duration.ofSeconds(30))\n .build());\n SpeechSettings speechSettings = speechSettingsBuilder.build();\n </code></pre>"
summary: "Settings class to configure an instance of <xref uid=\"com.microsoft.samples.google.SpeechClient\" data-throw-if-not-resolved=\"false\">SpeechClient</xref>.\n\n <p>The default instance has everything set to sensible defaults:\n\n <ul>\n <li>The default service address (speech.googleapis.com) and default port (443) are used.\n <li>Credentials are acquired automatically through Application Default Credentials.\n <li>Retries are configured for idempotent methods but not for non-idempotent methods.\n </ul>\n\n <p>The builder of this class is recursive, so contained classes are themselves builders. When\n build() is called, the tree of builders is called to create the complete settings object.\n\n <p>For example, to set the total timeout of recognize to 30 seconds:\n\n <pre class=\"prettyprint lang-java\"><code>\n SpeechSettings.Builder speechSettingsBuilder = SpeechSettings.newBuilder();\n speechSettingsBuilder\n .recognizeSettings()\n .setRetrySettings(\n speechSettingsBuilder\n .recognizeSettings()\n .getRetrySettings()\n .toBuilder()\n .setTotalTimeout(Duration.ofSeconds(30))\n .build());\n SpeechSettings speechSettings = speechSettingsBuilder.build();\n </code></pre>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The uid of com.microsoft.samples.google.SpeechClient seems incorrect in both before and after. Why?

Screen Shot 2022-06-17 at 4 24 20 PM

syntax:
content: "public class SpeechSettings extends ClientSettings<SpeechSettings>"
inheritance:
Expand Down