Project

Profile

Help

Support #6399

open

Is XmlProcessingError.IsWarning supposed to be "false" for SXWNxxxx?

Added by Emanuel Wlaschitz 3 months ago. Updated 3 months ago.

Status:
In Progress
Priority:
Low
Assignee:
-
Category:
.NET API
Sprint/Milestone:
-
Start date:
2024-04-16
Due date:
% Done:

0%

Estimated time:
Legacy ID:
Applies to branch:
10
Fix Committed on Branch:
Fixed in Maintenance Release:
Platforms:
.NET

Description

I know that Saxon 10 is pretty much on life-support at this point and unlikely to see another release beyond 10.9 (hence why I tagged this as "Support" rather than "Bug") but I just stumbled over one of the last tools that was still on Saxon 9.9: A tiny console application that runs in our CI to ensure we don't forget about other affected XSLT when re-using/changing modules.

I basically spin up a XSLT Compiler, compile a list of defined root XSLTs (files that include others thru xsl:import/xsl:include) and capture any exceptions/errors that come out:

var processor = new Processor();
var compiler = processor.NewXsltCompiler();
var staticErrors = new List<XmlProcessingError>();
compiler.SetErrorList(staticErrors);

// try/catch/etc. omitted for brevity
_ = compiler.Compile(GetStyleURI("validate.xsl"));

foreach (var staticError in staticErrors)
{
    var console = staticError.IsWarning ? Console.Out : Console.Error;
    console.WriteLine("[{0}] {1} ({2})",
        staticError.IsWarning ? "WARN" : "ERROR",
        GetMessage(staticError),
        GetLocation(staticError));
}

With 10.9 (from NuGet) every single staticError has IsWarning set to false, even the ones that used to be warnings with 9.9 (basically anything in the SXWNxxxx range.) At a glance, this feels like an oversight; or a gap in documentation elsewhere (at least I couldn't find anything for this.)

I see that AsWarning (not IsWarning) exists as well, but that one just gives me a copy that has IsWarning blindly set to true.

Is there any way to figure out if something is a warning (something that should be addressed while the XSLT still works) and not a hard error (something that normally causes the XSLT to fail completely) without basically testing for staticError.ErrorCode.LocalName.StartsWith("SXWN") in my code? I can imagine there are other codes that aren't technically errors (that would fail this test); and not every StaticError has an ErrorCode (one that we tend to run into is "Stylesheet module included.xsl is included or imported more than once. This is permitted, but may lead to errors or unexpected behavior")

Again, mostly looking for ideas on how to get this little tool onto 10.9 so it matches the rest of our landscape (and may support those that run on it) without causing too much of a ruckus. I don't really expect to see a 10.10 maintenance release to address this (although, it would be 10/10 if there was one :) )


Files

clipboard-202404250755-c072v.png (61 KB) clipboard-202404250755-c072v.png Screenshot from the Visual Studio Debugger Emanuel Wlaschitz, 2024-04-25 07:55
Actions #1

Updated by Michael Kay 3 months ago

  • Status changed from New to AwaitingInfo

I haven't gone back to the Saxon10 code base, but I've done a quick survey of the current development branch, and I haven't found any cases where SXWN codes are used other than on warnings. If you can identify specific cases, please let me know.

Actions #2

Updated by Emanuel Wlaschitz 3 months ago

Ah, great, the attachment didn't make it. My apologies.

Here's validate.xsl:

<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:x="urn:x" version="3.0">
  <xsl:template match="/">
    <root>
      <xsl:variable name="case">three</xsl:variable>
      <xsl:if test="case">
        <xsl:attribute name="case">
          <xsl:value-of select="x:test($case)"/>
        </xsl:attribute>
      </xsl:if>
      <xsl:for-each select="."/>
      <xsl:variable name="unused">variable</xsl:variable>
    </root>
  </xsl:template>
  <xsl:function name="x:test" as="xs:integer?">
    <xsl:param name="input"/>
    <xsl:if test="$input castable as xs:integer">
      <xsl:value-of select="$input"/>
    </xsl:if>
  </xsl:function>
</xsl:stylesheet>

This logs the following:

[ERROR] SXWN9009: An empty xsl:for-each instruction has no effect (validate.xsl line 10 column 33)
[ERROR] SXWN9001: A variable with no following sibling instructions has no effect (validate.xsl line 11 column 35)

The [ERROR] prefix is derived from !staticError.IsWarning (as evident in the code snippet above.) Screenshot from the Visual Studio Debugger

I was also expecting the xsl:variable/xsl:if to warn me about a potential mismatch (and that I should use child::case to make it obvious I mean the element, not the variable or an operator) and the xsl:attribute/xsl:value-of about using the @select attribute directly...but I'm not sure where they are. Maybe our bigger XSLTs have something else that makes those trigger with Saxon 9:

[WARN] SXWN9000: in {mod[}: The keyword 'mod' in this context means 'child::mod'. If this was intended, use 'child::mod' or './mod' to avoid this warning. (common/schedule.xsl line 819)
[WARN] SXWN9001: A variable with no following sibling instructions has no effect (common/tocs.xsl line 161 column 73)
[WARN] SXWN9000: A function that computes atomic values should use xsl:sequence rather than xsl:value-of (common/functions.xsl line 1014)

That is using the exact same code (on Saxon-HE 9.9.1.8 on .NET FRamework 4.8,) and comes back was [WARN] (with any error output on stderr failing the CI run.)

Actions #3

Updated by Michael Kay 3 months ago

  • Status changed from AwaitingInfo to In Progress
Actions #4

Updated by Michael Kay 3 months ago

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.

Actions #5

Updated by Emanuel Wlaschitz 3 months ago

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!

Actions #6

Updated by Michael Kay 3 months ago

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.

Actions #7

Updated by Emanuel Wlaschitz 3 months ago

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 :)

Actions #8

Updated by Emanuel Wlaschitz 3 months ago

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.)

Actions #9

Updated by Michael Kay 3 months ago

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()?

Actions #10

Updated by Emanuel Wlaschitz 3 months ago

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.)

Actions #11

Updated by Emanuel Wlaschitz 3 months ago

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

Actions #12

Updated by Michael Kay 3 months ago

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.

Actions #13

Updated by Emanuel Wlaschitz 3 months ago

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.)

Please register to edit this issue

Also available in: Atom PDF