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

Add method signatures for JDK >= 11 #184

Closed
Marcono1234 opened this issue Nov 5, 2021 · 8 comments · Fixed by #195
Closed

Add method signatures for JDK >= 11 #184

Marcono1234 opened this issue Nov 5, 2021 · 8 comments · Fixed by #195
Assignees
Milestone

Comments

@Marcono1234
Copy link

Hello,
the following methods have been added in or after JDK 11. Would it make sense to consider them?

For the following methods added in JDK 17 I am not sure if they should be considered. For the Standard In and Out and Error steams it might make sense to use the native / default encoding; though if the other process explicitly specifies an encoding, that should of course be used.

I found these methods using the newly added "New" tab of the Javadoc: https://docs.oracle.com/en/java/javase/17/docs/api/new-list.html (see JDK-8269400).
Maybe that can be useful for you in the future as well.
However, this information is based on the @since tag. So if the JDK authors forget it you will miss APIs. It apparently also only considers direct usage of @since. So if a class is annotated with @since, but its methods are not, then only the class is listed as new.

In case you were not aware of it already, there are also https://github.com/AdoptOpenJDK/jdk-api-diff and https://github.com/marchof/java-almanac/; but for both the output is rather verbose given that mainly newly added methods and contructors are relevant for this project.

@uschindler
Copy link
Member

uschindler commented Nov 5, 2021

Hi,

Hello, the following methods have been added in or after JDK 11. Would it make sense to consider them?

I will check those, thanks for reporting!

For the following methods added in JDK 17 I am not sure if they should be considered. For the Standard In and Out and Error steams it might make sense to use the native / default encoding; though if the other process explicitly specifies an encoding, that should of course be used.

Those is indeed worth a discussion. But other methods like System.in which is a printstream also uses default encoding, same for the Readers and Writers. For the external process this is a bit different (because we call it), but I think most uses of those methods are about communicating with console programs).

A good idea would be to check the latest JDK 18 JEP about making UTF-8 the default (unless specified differently). It would be good to figure out how this is handled in this issue.

I will dig, but I am a bit busy at moment.

@uschindler uschindler self-assigned this Nov 5, 2021
@uschindler uschindler added this to the 3.3 milestone Nov 5, 2021
@Marcono1234
Copy link
Author

A good idea would be to check the latest JDK 18 JEP about making UTF-8 the default (unless specified differently). It would be good to figure out how this is handled in this issue.

The documentation of these methods say they use the native.encoding System property. If I understand JEP 400 and JDK-8266075 correctly that System property will have the value Charset.defaultCharset() used to have.
For console applications I assume using the OS specific charset would make sense, however I am not sure if that makes sense when communicating with other processes.

@Marcono1234
Copy link
Author

Marcono1234 commented Nov 6, 2021

Maybe the following < JDK 11 methods should be added as well (probably not worth it to create a separate GitHub issue):

Note that the signatures above don't exactly match the ones used by this project; they have to be adjusted.

In case you are interested, I have written a small CodeQL query which can be used to find callables using default Charset or Locale. You can use it on https://lgtm.com to run against public GitHub projects, for example Apache Commons IO. It might however produce some false positives and have some false negatives.

CodeQL query
import java

class CharsetOrLocaleParam extends Parameter {
    CharsetOrLocaleParam() {
        getType().hasName(["Charset", "Locale"])
        or
        // Or is charset name as String
        getType() instanceof TypeString
        and exists(getName().regexpFind("(?i)(charset|encoding)", _, _))
    }
}

predicate usesDefault(Callable c) {
    // Either Javadoc mentions default Charset or Locale
    exists(string javadoc |
        // Join Javadoc lines using " "
        javadoc = concat(JavadocText javadocText |
            javadocText.getJavadoc().getCommentedElement() = c
        |
            javadocText.getText(), " " order by javadocText.getLocation().getStartLine() asc
        )
        and exists(javadoc.regexpFind("(?i)(default|platform)\\s+(locale|charset|encoding)", _, _))
        and not c.getAParameter() instanceof CharsetOrLocaleParam
    )
    // Or overload with Charset or Locale parameter exists
    or exists(Callable overload |
        overload.getDeclaringType() = c.getDeclaringType()
        and overload.getName() = c.getName()
        and overload != c
        and (
            // Either has no parameters
            c.hasNoParameters()
            and overload.getAParameter() instanceof CharsetOrLocaleParam
            or
            // Or has same parameters except for Charset / Locale parameter
            exists (int specificIndex |
                overload.getNumberOfParameters() = c.getNumberOfParameters() + 1
                and overload.getParameter(specificIndex) instanceof CharsetOrLocaleParam
                and forall(int smallerIndex |
                    smallerIndex = [0..specificIndex - 1]
                |
                    c.getParameterType(smallerIndex) = overload.getParameterType(smallerIndex)
                )
                and forall(int largerIndex |
                    largerIndex = [specificIndex + 1..overload.getNumberOfParameters() - 1]
                |
                    c.getParameterType(largerIndex - 1) = overload.getParameterType(largerIndex)
                )
            )
            // Ignore false positives from detection above
            and not c.getAParameter() instanceof CharsetOrLocaleParam
        )
        // Ignore if documentation says default is UTF-8 (e.g. java.nio.file.Files methods)
        and not exists(JavadocText javadocText |
            javadocText.getJavadoc().getCommentedElement() = c
            and exists(javadocText.getText().regexpFind("(?i)UTF(\\-| )8", _, _))
        )
    )
}

// Build signature in the format specified by https://github.com/policeman-tools/forbidden-apis/wiki/SignaturesSyntax
string getQualifiedTypeName(Type t) {
    if t instanceof Array then result = getQualifiedTypeName(t.(Array).getElementType()) + concat(int i | i = [1..t.(Array).getDimension()] | "[]")
    else if t instanceof RefType then result = t.(RefType).getSourceDeclaration().getQualifiedName()
    else result = t.getName()
}

string getParameters(Callable c) {
    result = "(" + concat(Parameter p |
        p = c.getAParameter()
    |
        getQualifiedTypeName(p.getType()), "," order by p.getPosition() asc
    ) + ")"
}

string getQualifiedName(Callable c) {
    exists(string callableName |
        result = c.getDeclaringType().getQualifiedName() + "#" + callableName + getParameters(c)
    |
        c instanceof Method and callableName = c.getName()
        or c instanceof Constructor and callableName = "<init>"
    )
}

from Callable c, string qualifiedName
where
    c.fromSource()
    // Only consider publicly visible callables
    and (c.isProtected() or c.isPublic())
    and forall(RefType t |
        t = c.getDeclaringType().getEnclosingType*()
    |
        t.isProtected() or t.isPublic()
    )
    and not c.getDeclaringType() instanceof TestClass
    and usesDefault(c)
    and qualifiedName = getQualifiedName(c)
// IMPORTANT: qualifiedName might not match exactly the format used by https://github.com/policeman-tools/forbidden-apis
select qualifiedName, c order by qualifiedName

@uschindler
Copy link
Member

I am working on the process to releasea new version with JDK 18 support, so I will look into this today. Adding the signatures should be easy.

@uschindler
Copy link
Member

Hi,

I stepped through the whole list and added signatures in 1.7 file (the very old ones), but left out the following:

I also left out the Process readers/writers for the reasons described above. Also with the new UTF-8 as default change in later Java versions those still use the system charset.

@uschindler
Copy link
Member

Here is the PR: #195

Please have a look at it, if all is fine I would merge and then release version 3.3.

@uschindler
Copy link
Member

P.S.: If you have some addition, now is last chance!

@Marcono1234
Copy link
Author

Looks good, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants