Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(xml-views-validation): avoid false positive errors for unknown tags #144

Closed
wants to merge 1 commit into from

Conversation

bd82
Copy link
Member

@bd82 bd82 commented May 24, 2020

If an unknown tag was under a none namespace UI5 entity fqn, e.g the FQN of an enum/interface it
would be treated as a valid known namespace and thus an error would occur.

If an unknown tag was under a none namespace UI5 entity fqn, e.g the FQN of an enum/interface it
would be treated as a valid known namespace and thus an error would occur.
@bd82
Copy link
Member Author

bd82 commented May 24, 2020

failure on nodejs 10 is unrelated to this change, Seems master is failing too with some timeout

xmlns:mvc="sap.ui.core.mvc"
<!-- The error should be on the definition of an invalid xmlns which is not a UI5Namespace rather than on each usage -->
xmlns:AvatarColor="sap.m.AvatarColor">
<AvatarColor:Green>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why shouldn't this give an error? Green isn't a class (or aggregation), it can't appear in a tag name. An error here is not a false positive. It would fail in runtime.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had xmlns:bamba="sap.m.Bamba" would we have given an error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also there is only one error here imho, the invalid namespace.
Every usage of the namespace in a tag should not be an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had xmlns:bamba="sap.m.Bamba" would we have given an error?

We would have given a warning on the namespace because we don't know it and it's in an sap library namespace (sap.m). We would not have given an error (or done validations, or code assist) on tags that use this namespace because we consider it to be a custom namespace.

This isn't the case when you put an existing symbol name in the xmlns attribute. That is known (so we can't say it's an "unknown namespace"). It cannot be considered a custom namespace (so we cannot treat tags that use it as possible custom controls that should be excluded from validations). And it might have classes inside it which we should validate and suggest for code completion.

@bd82
Copy link
Member Author

bd82 commented Aug 12, 2020

No capacity to continue with this, closing at this time. It will be tracked in #262

@bd82 bd82 closed this Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants