RFR [14] 8234746: Improve indexing of system properties

[Jonathan Gibbons]

On 11/25/2019 09:19 AM, Pavel Rappo wrote:
> Hello,
>
> Please review the following change for https://bugs.openjdk.java.net/browse/JDK-8234746:
>
>     http://cr.openjdk.java.net/~prappo/8234746/webrev.00/
>
> Before:
>
> javax.rmi.ssl.client.enabledCipherSuites - Search tag in javax.rmi.ssl.SslRMIClientSocketFactory
> javax.rmi.ssl.client.enabledCipherSuites - Search tag in javax.rmi.ssl.SslRMIClientSocketFactory
> ...
> file.separator - Search tag in java.lang.System
>
> After:
>
> javax.rmi.ssl.client.enabledCipherSuites - Search tag in javax.rmi.ssl.SslRMIClientSocketFactory
> javax.rmi.ssl.client.enabledCipherSuites - Search tag in javax.rmi.ssl.SslRMIClientSocketFactory.createSocket
> ...
> file.separator - Search tag in java.lang.System.getProperties
>
> Thanks,
> -Pavel
>

You either need a test or mark the bug as noreg-something.


Uugh.

I agree that the fix will work, but I cringe at the methods in Utils 
that make you fix it this way.

Readingline 441 of your fix, it is "obviously wrong" since the simple 
name of an element is generally the last component of the qualified 
name, but I see that "Utils.getFullyQualifiedName" will (and is 
specified to) return the qualified name of the enclosing element in some 
cases, which is not an intuitive definition based on the method's name.

I guess I wonder how often Utils.getFullyQualifiedName is invoked with a 
field or method, and whether we should make those use sites use 
Utils.getFullyQualifiedName(e.getEnclosingElement()) if they really want 
the qualified name of the enclosing element, so that 
Utils.getFullyQualifiedName() can throw ILA if called with an element 
that does not have a fully qualified name.

Proactively applying that logic to your fix, it would be less jarring if 
you were to change it to:

- 441 si.setHolder(utils.getFullyQualifiedName(e) + "." + 
utils.getSimpleName(e)); + 441 
si.setHolder(utils.getFullyQualifiedName(e.getEnclosingElement()) + "." 
+ utils.getSimpleName(e));


That being said, I see your fix is the same as in line 435, which 
suggests we might want to modify that line as well to better reflect the 
notions of fully qualified name and simple name.  Curiously, the code 
for visitVariable "almost" gets it right, by getting the enclosing type 
element (te) but then it ignores it.

--Jon






-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20191127/f5d21faa/attachment.html>