Change XML output file's version to 1.1

[DReigada]

What version are you using?
4.7.1 - But this applies to all versions

What did you do?

1. Have a file with an object like this: var foo = { "\b":42 };
2. Run eslint with the -f checkstyle flag

What happened?
One of the errors is:
<error line="1" column="18" severity="error" message="Missing space before value for key &apos;&#8;&apos;. (key-spacing)" source="eslint.rules.key-spacing" />

This XML is valid for version 1.1, but the XML header has the version set to 1.0, in which some character references like  are not valid.

What did you expect to happen?

The XML version on the output file should be 1.1

I think this might be related to #6673.

[platinumazure]

I'm not saying this is likely, but it's possible some users are depending on us outputting 1.0 right now and so changing to 1.1 could be a breaking change.

Is there a way to solve the entity issue in 1.0, so that we could land that change as a patch and then we could consider outputting 1.1 in a major release (if it is indeed considered a breaking change)?

[DReigada]

I don't see another way of solving this, since version 1.0 does not support those characters.
Unless those characters are filtered before generating the XML, which can be a problem since that would change the messages' original content.

[not-an-aardvark]

I'm not very familiar with XML versioning. How would outputting 1.1 be a breaking change?

[platinumazure]

I'm not saying it would be for sure, I was just asking the question.

For humans reading XML, it definitely wouldn't be breaking.

But if the lint output is itself read by some other system, and that system is old and only understands 1.0 semantics and throws if it gets 1.1 XML, then that could inconvenience users, although it's an integration breaking rather than core functionality.

Actually there would be two scenarios depending on whether entities (described in this bug fix) are output:

• If lint results already include entities, then it's already invalid XML and ether the user's integration is already complaining, or it's handling the invalid output fine. This isn't a breaking change.
• If a user does not have entities in their XML output, then switching to 1.1 could be a breaking change if their integration outright refuses to read XML 1.1.

I'm more concerned about the second group.

I don't know how realistic this is. In fact, I doubt many users would be affected. But we've put breaking labels on more trivial changes.

[not-an-aardvark]

I guess we could handle both groups by switching to XML 1.1 only when the output contains entities. But that seems like a hacky workaround.

[platinumazure]

I'd be okay with maybe accepting this change, and releasing a new formatter with the old behavior (e.g., "xml10") if users complain.

[not-an-aardvark]

Sounds good to me. Removing the "breaking" label and marking this as accepted.

[Janiczek]

Just came to chime in that this indeed broke our Jenkins:

12:40:38 ERROR: Step ‘Report Violations’ aborted due to exception: 
12:40:38 org.xmlpull.v1.XmlPullParserException: only 1.0 is supported as <?xml version not '1.1' (position: START_DOCUMENT seen <?xml version="1.1"... @1:19) 

We're currently sed -i-ing the resulting XML report file to change it back to 1.0, but ... yeah, not ideal.

[platinumazure]

Reopening since we need to revert the PR due to breaking integrations.

I was thinking about this and I think the correct approach is actually to fix the error messages.

If a rule is reporting on an object property such as foo["\b"], it is true that the computed property is using an escape sequence that is unprintable. However, ESLint should represent this error using the text the user typed: In other words, the error message should actually have a literal backslash and then the letter b (or, as a JS string, "\\b").

We can usually abstract this using sourceCode.getText() and other methods on that object, which focus on the parsed source text.

Please elaborate on the lint error (rule, source code, and error message). We should fix the lint message, so that the error message is compatible with XML-based formatters. Thanks!

[j-f1]

Here’s an example in the online demo with U+0008 as part of the message.

[platinumazure]

Can we do so without breaking those old Jenkins installations (with no way to fix it)? If not, I think we should consider creating an xml11 formatter instead and deprecate the xml formatter.

…

On Jan 12, 2018 4:58 PM, "Teddy Katz" ***@***.***> wrote: Should we consider applying this as a breaking change next major release? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#9607 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARWesbiuqEW6ubKetLBmnnJGqOpPgZ_ks5tJ-OcgaJpZM4QZrEs> .

[not-an-aardvark]

Making a new formatter also sounds good to me.

[j-f1]

What about the opposite — updating the xml formatter and providing a backwards-compatible formatter?

[platinumazure]

@j-f1

What about the opposite — updating the xml formatter and providing a backwards-compatible formatter?

If done in a major release, that would be fine too.

[platinumazure]

I'm 👎 to changing the formatter to XML 1.1 without providing an alternative. My preference would be to leave the current formatter alone and create a new one. This would allow us to tackle this outside of a major release, as well-- I'm not sure this necessarily needs to be done in 5.0.

[platinumazure]

TSC Summary: The messages that are part of the current xml formatter output have been identified in some cases as being non-compliant with XML 1.0 standards (having to do with entity encoding, but I can't remember the details). We decided to change the XML version to 1.1, but that broke some people's integrations which could only handle XML 1.0. We could set XML to 1.1 as a breaking change, or we could create a new formatter and thus support both. One possibility that avoids the need for aligning with a major release is to leave the current xml formatter as 1.0 and create an xml11 formatter that outputs as XML 1.1.

TSC Questions:

• Should we be making a change in the 5.0 release?
• Should we continue to support XML 1.0 output?
• Should we support XML 1.1 output?
• Should we create a new formatter and/or change the existing formatter, and which version should each formatter support?

[alberto]

I'd vote to add a new formatter and leave the old one as is.

[not-an-aardvark]

In today's TSC meeting, the TSC decided to update the existing xml formatter to support XML 1.1 in a major release, and to add a new xml-1.0 formatter with the legacy behavior.

[alberto]

@eslint/eslint-tsc After having a look at this, we framed the discussion around a non-existent xml formatter. The actual formatters that use xml output are checkstyle, jslint-xml and junit. I am a bit more reluctant to version these according to xml version. I propose we revisit the decision taking this into account.

[mysticatea]

My understanding is:

1. We update all of checkstyle, jslint-xml and junit to XML 1.1.
2. We add checkstyle-1.0, jslint-xml-1.0 and junit-1.0 for compatibility. Most people must not care those formatters.

Indeed, *-1.0 may be unnecessary since people can make custom formatters.

[alberto]

After having a look around, it seems XML 1.1 is not widely adopted, so I'm unsure if we should even make the change or recommend a custom formatter for those who need it.

[not-an-aardvark]

In today's TSC meeting, the TSC decided to not make any change to existing formatters. Users who need XML 1.1 for their results are encouraged to create a custom formatter.