Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [el-dev] Wrong TCK test for ambiguous methods?

Hi,

On Wed, Apr 13, 2022 at 10:04 AM Mark Thomas <markt@xxxxxxxxxx> wrote:
Hi Arjan,

EL method matching includes an extra step compared to normal Java
matching as it also uses EL coercion. This does make things more
complicated as it introduces quite a few additional edge cases.

[...]

Since there is no way do disambiguate these options (both have one
matching type and one coerced type) the match is ambiguous.

Therefore, I think this test is correct.

To be extra sure, I just debugged in Tomcat, (M14), using:

<dependency>
    <groupId>org.apache.tomcat</groupId>
    <artifactId>tomcat-jasper-el</artifactId>
    <version>10.1.0-M14</version>
</dependency>

But it indeed triggers a method not found internally. 

In reflectionUtil, it eventually executes this test:

  if (mParamTypes[i].equals(paramTypes[i])) {
                        exactMatch++;
                    } else if (paramTypes[i] != null && isAssignableFrom(paramTypes[i], mParamTypes[i])) {
                        assignableMatch++;
                    } else {

It only checks for assignability, not coercion. 

So when it finds the "public String targetE(Long, Long)" method, it checks whether String can be assigned to Long using java.lang.Class.isAssignableFrom(Class<?>), which fails. When it encounters "public String targetE(String, String)" the first parameter is an exact match, but for the second parameter it checks whether Long can be assigned to String, which also fails.

Eventually this part is executed:

// Handle case where no match at all was found
        if (match == null) {
            throw new MethodNotFoundException(MessageFactory.get(
                        "error.method.notfound", base, property,
                        paramString(paramTypes)));

So I don't see any ambiguity happening here. Is the coercion missing in the Tomcat code perhaps?

Kind regards,
Arjan Tijms
 

Back to the top