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.

Sign up for GitHub

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

Jump to bottom

NPE with TypeDefinition #771

Merged
merged 1 commit into from
Jun 10, 2020
Merged

NPE with TypeDefinition #771

merged 1 commit into from
Jun 10, 2020

Conversation

angelozerr
Copy link
Contributor

Fixes #629

Signed-off-by: azerr [email protected]

@angelozerr angelozerr force-pushed the typedefinition-npe branch from 5f44942 to 7bfb0d9 Compare June 9, 2020 10:18
@angelozerr
Copy link
Contributor Author

Please not this PR is not perfect. When a xs:element is not included inside a xs:complexType I search the first xs:element (xs:element/@name) which matches the origin element local name which can be wrong.

@angelozerr angelozerr force-pushed the typedefinition-npe branch 2 times, most recently from 332da84 to c4d03c7 Compare June 10, 2020 11:57
@angelozerr angelozerr marked this pull request as ready for review June 10, 2020 11:58
@angelozerr angelozerr force-pushed the typedefinition-npe branch 2 times, most recently from d1b9120 to 038f5ac Compare June 10, 2020 12:11
@angelozerr
Copy link
Contributor Author

The PR is ready and tests are written. The PR should work now correctly and jump every time to the proper xs:element declaration.

The main idea of find type location is to find the location of the Xerces XSElementDeclaration. It works (in master) for global xs:element or local xs:element which are declared inside a xs:complexType but not when xs:element is declared inside a xs:choice, xs:sequence, etc it doesn't work. This PR fixes the problem.

To test this PR you can

  • Test 1. follow the explanation at NPE with TypeDefinition #629 (comment)
  • Test 2. open the Format.xml (which belong to this PR) and find type definition for NameProperty which is declared in Format.xsd inside a xs:choice and not in xs:complexType:
<xs:group name="Expression">
    <xs:choice>
      <xs:element name="PropertyName" type="xs:string" />
...    </xs:choice>
  </xs:group>

@angelozerr angelozerr force-pushed the typedefinition-npe branch from 038f5ac to 7e00698 Compare June 10, 2020 14:19
fbricon
fbricon reviewed Jun 10, 2020
View reviewed changes
...pse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDDocument.java
Comment on lines +434 to +465
// - fParticle array of XSParticleDecl where fValue is an instance of
// XSElementDeclaration
// - fLocalElementDecl array of Xerces Element which stores the element offset.

// Get the XMLSchemaLoader instance from the XSLoader instance
Field f = XSLoaderImpl.class.getDeclaredField("fSchemaLoader");
f.setAccessible(true);
XMLSchemaLoader schemaLoader = (XMLSchemaLoader) f.get(xsLoader);

// Get the XSDHandler instance from the XMLSchemaLoader instance
Field f2 = XMLSchemaLoader.class.getDeclaredField("fSchemaHandler");
f2.setAccessible(true);
XSDHandler handler = (XSDHandler) f2.get(schemaLoader);

// Get the XSParticleDecl array from the XSDHandler instance
Field f3 = XSDHandler.class.getDeclaredField("fParticle");
f3.setAccessible(true);
XSParticleDecl[] fParticle = (XSParticleDecl[]) f3.get(handler);

// Get the index where elementDeclaration is associated with a XSParticleDecl
int i = getXSElementDeclIndex(elementDeclaration, fParticle);
if (i >= 0) {
// Get the Xerces Element array from the XSDHandler instance
Field f4 = XSDHandler.class.getDeclaredField("fLocalElementDecl");
f4.setAccessible(true);
Element[] fLocalElementDecl = (Element[]) f4.get(handler);
return (ElementImpl) fLocalElementDecl[i];
}
} catch (Exception e) {
LOGGER.log(Level.SEVERE,
"Error while retrieving mapped Xerces xs:element of '" + elementDeclaration.getName() + "'.", e);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

My eyes burn from all that reflection

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know -( But it's the only mean that I have found to get those Xerces information. The master code uses already reflection for the xs:complexType case.

@angelozerr angelozerr force-pushed the typedefinition-npe branch 2 times, most recently from 398c82a to d01ea4e Compare June 10, 2020 15:31
@xorye
Copy link

xorye commented Jun 10, 2020

I'm good with merging this PR

@xorye xorye merged commit e564c60 into eclipse-lemminx:master Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xorye xorye xorye left review comments

@fbricon fbricon fbricon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NPE with TypeDefinition
3 participants
@angelozerr @xorye @fbricon