Saxon

Thanks for the repro.

I've checked in the Java code and that is definitely marking this correctly as a warning. I'll now need to check it on .NET, for which I need a different debugging setup.

Interresting. I might be able to rewrite this tool in Java if this is indeed an issue on the .NET API; there's not a whole lot going on with it. The only drawback would be that I might run into other issues that only affect the .NET API (since our tools run on .NET, making the validation use the same libraries just makes sense in my head.)

Thanks for looking at this despite the age and status of Saxon 10!

I don't think we have ever produced warnings when you use an element name like case in place of the attribute name $case (alternatively it's possible that we tried doing it and it caused too much annoyance).

And I think we produce the warning about using xsl:sequence rather than xsl:value-of to deliver the function result only if the instruction is actually the last thing in the function body, not if it's contained in xsl:if.

Yeah, that might be on me (for not looking at the actual XSLT that triggers the warning in CI.)

Over there it is an xsl:apply-templates with alternatives, so I could see the word case be mistaken for an XQuery expression (just like the mod could be.)

<!--
[ERROR] SXWN9000: in {if | else |}: The keyword 'else' in this context means 'child::else'. If this was intended, use 'child::else' or './else' to avoid this warning. (validate.xsl line 11 column 5)
[ERROR] SXWN9000: in {if | else | case}: The keyword 'case' in this context means 'child::case'. If this was intended, use 'child::case' or './case' to avoid this warning. (validate.xsl line 11 column 12)
-->
<xsl:apply-templates select="if | else | case"/>

Same for the xsl:function, this one does produce the warning:

<!--
[ERROR] SXWN9000: A function that computes atomic values should use xsl:sequence rather than xsl:value-of (validate.xsl line 21 column 48)
-->
<xsl:function name="x:depth" as="xs:integer">
  <xsl:param name="context"/>
  <xsl:number value="count($context/ancestor-or-self::*[self::section or self::chapter])"/>
</xsl:function>

But, again, my main problem right now is that staticError.IsWarning is never set; not that my attempts to produce a working test case from memory are failing :)

Ah yes, that's where my incorrect guess came from: Oxygen XML Developer also runs a schematron called xsltBadPractices.sch as part of the validation, which includes this one (amongst others that I tried to pin on Saxon):

System ID: C:\_git\dev\SaxonTests\SaxonTests\Testdata\styles\validate.xsl
Main validation file: C:\_git\dev\SaxonTests\SaxonTests\Testdata\styles\validate.xsl
Scenario name: XSLT Bad Practices
Schema: C:\Program Files\Oxygen XML Editor 26\frameworks\xslt\sch\xsltBadPractices.sch
Document type: XSLT
Engine name: ISO Schematron
Severity: warning
Problem ID: xsltBadPractices.sch:conflictingNamesNoInVariablesID
Description: The XPath expression references the 'case' element(s), but there are already parameters/variables with the same name declared in the current context. It is recommended to use different names to avoid potential problems.
Start location: 5:27

So yeah, I'm mainly interrested in actual compiling errors that would fail 100% of cases, while warnings simply go to the log and are up to the developer to address when there's time (or reason) to do it. Quality is also important, but not in a way that it should fail CI (yet.)

I looked back at the C# code for version 10 and yes, it's not setting the IsWarning property on class StaticError.

Probably the constructor for StaticError should set this property based on the isWarning() property of the underlying XPathException object. However, I'm not going to attempt to revisit the Saxon 10 build to make and test this change.

Can you perhaps get the information by calling StaticError.UnderlyingException.isWarning()?

I don't see StaticError.UnderlyingException.isWarning() in the debugger, and isSyntaxError(), isReportableStatically(), isGlobalError(), isStaticError() and isTypeError() all return false. I don't mind reaching into the underlying Java code (as exposed by IKVM) for this particular case, but it seems that this info isn't available to me.

From what I could see in the disassembly of the StaticError ctor, the passed Java Exception is likely an XPathException already, since there is no InnerException on it (which would suggest it going through XPathException.makeXPathException instead.)

This is with Saxon-HE 10.9.0 from NuGet, just in case it matters here.

OK. (Sorry, I'm doing this without a Windows machine so I can see the code but can't run it).

If you can set an ErrorReporter on the XsltCompiler, then instead of constructing an ErrorList it will notify each error individually as an instance of XmlProcessingError, which wraps the Java object net.sf.saxon.s9api.XmlProcessingError which should have an isWarning() method.

Yeah, that one has a private field called error which has the isWarning() method with the correct value. After looking back, the ErrorList also has it. No accessor though, at least I don't see one. I can just use reflection on it, since I don't expect any sudden changes in version 10 that might break this.

This seems to work (in case anyone else needs it):

private static bool IsWarning(XmlProcessingError staticError)
{
    if (staticError.IsWarning)
        return true;

    // StaticError has the underlying XmlProcessingError; but it isn't public.
    var errorMember = staticError.GetType().GetField("error", BindingFlags.Instance | BindingFlags.NonPublic);
    if (errorMember?.GetValue(staticError) is net.sf.saxon.s9api.XmlProcessingError xmlError)
        return xmlError.isWarning();

    // fallback, just in case.
    if (staticError.ErrorCode != null && staticError.ErrorCode.LocalName.StartsWith("SXWN"))
        return true;

    return false;
}

I can live with that, thanks for your help!

PS: Any news or changes on the licensing front for SaxonCS? There wasn't a whole lot of movement in the other ticket, other than questions if it would be on the version 13 roadmap (in terms of a PE and HE version.)