"Error" severity for server.xml config element seems too high, especially when the element would be valid if additional features were activated.

[scottkurz]

There are three cases to consider in this issue:

1. The case that a config element is invalid and would be unrecognized even by the maximal, cached XSD

   • E.g. <dummyValue/>.

2. The case that a config element is valid per the XSD generated for the specific installation (in target/.libertyls) but the corresponding feature is not activated.

   • E.g. if I used the full 'openliberty-runtime' install artifact and used the <batchPersistence/> element without one of the batch* features configured in server.xml.

3. The case where a config element is invalid per the install-generated XSD, but would be valid per the cached XSD.

   • E.g. the same <batchPersistence/> element starting from just the kernel artifact without any batch features yet installed.

From the runtime perspective, an unknown element will generally be ignored. It's something to be aware of but it seems hard to justify as a true error?

If we wanted to change the severity but otherwise keep our existing approach using the gen'd vs. cached XSD, I think we'd handle each of these like:

1. Can we configure lemminx to lower the severity of an XSD validation diagnostic from ERROR->WARNING?
2. This is entirely implemented by LCLS, so we can easily change this in this repo.
3. The interaction btw. the IDE tools and LMP/LGP mostly eliminate this scenario. If dev mode is up and running when the change is made, the new feature will be installed and the XSD re-generated. It's the user's burden to understand enough of the tooling workflow to deal with this in the "gap" here (e.g. if a feature is added when dev mode is not running... see https://github.com/OpenLiberty/liberty-language-server/issues/252). So at this point, what's left then basically collapses into case 1., can we lower lemminx's XSD invalid severity level?

Having said all that, it's worth noting this is a place where it can be a bit confusing that the tools produce different diagnostic messages depending on whether you've started dev mode or not (mapping on to whether you're using a generated or cached XSD).

This would become even more noticeable, say, if we can't lower the XSD validation severity in 1. and the same time as in 2.... you could have a situation where the diagnostic appears one way when you first clone, then differently when you first start, then differently again if you did a clean.

It raises the question: should we consider validating against the cached XSD always ? Though that too has some downsides... what if the cached XSD isn't updated with some new (beta?) features that we could incorporated with the custom-generated XSD approach we used today? (Or with user-developed features).

OK, this is long enough for an initial comment. Will have to discuss more.

[cherylking]

@scottkurz At first I thought you were just proposing that we lower the severity from "Error" to "Warning" or even "Info" along with updating the message to make it clear that all may be fine (especially considering the fact we are not looking at other included server xml files for configured features). But your description covers a lot of different scenarios. So can we agree at least that this diagnostic should not be an "Error" and go ahead and make that change, but leave this issue open for further discussion?

As for always using the cached XSD, I'm afraid that doesn't make sense for a couple reasons:

1) It is for Open Liberty only. What about WebSphere Liberty? 2) It won't have the latest features introduced by newer runtimes. The cached schema will always lag by a few months.

[scottkurz]

So can we agree at least that this diagnostic should not be an "Error" and go ahead and make that change, but leave this issue open for further discussion?

That sounds good. Yes, this did sprawl into other related issues.

As for always using the cached XSD, I'm afraid that doesn't make sense for a couple reasons:

1. It is for Open Liberty only. What about WebSphere Liberty?
2. It won't have the latest features introduced by newer runtimes. The cached schema will always lag by a few months.

OK, I did have some thoughts here but realize I didn't list them.

First, I was assuming that OL's XSD is a subset of WL's XSD and so we can just validate against the broader WL XSD. So we'd have to abandon the idea of an OL-only validation. So it'd be debatable if we want that but that was my thought.

As far as 2., I was thinking we'd have to publish the latest XSD and code LCLS to go download the latest. I think we had talked about publishing the latest XSD in the past so that's debatable too.

However, there's another "newer runtime" variation that I'd missed or forgotten, which is the use of the beta. E.g. when the EE 10 features were added we could easily incorporate its beta features. It's striking me now as a bridge too far to have a superset XSD that also includes beta, and WL... every feature ever known to IBM.

So I think I'm circling back around to not being too crazy about this idea. Thanks for the discussion though.

[scottkurz]

@cherylking I just gave this another look after reading through the code to see how we generate the features list, e.g. starting from io.openliberty.tools.langserver.lemminx.services.FeatureService.getFeatures(String, String, int, String).

I see we release features.json artifacts for each of OL, WL, for each release version.

For beta, we generate a feature list (which we also do for older versions before we released a features.json).

While that obviously requires a whole separate release process to generate and release this XSD artifact... I wonder if we could at least stop and decide whether we think that's desirable? It seems to me like it might be the best compromise, taking for granted that releasing a versioned XSD or WL/OL each is feasible.