Skip to content

Property expression validation#21

Merged
angelozerr merged 1 commit intoeclipse-lsp4mp:masterfrom
datho7561:334-property-expression-validation
Sep 3, 2020
Merged

Property expression validation#21
angelozerr merged 1 commit intoeclipse-lsp4mp:masterfrom
datho7561:334-property-expression-validation

Conversation

@datho7561
Copy link
Copy Markdown
Contributor

@datho7561 datho7561 commented Jul 22, 2020

Two things are validated:

  • If there is a closing brace
  • If the referenced property exists (if it is defined in a Java file or assigned a value in the properties file)

Closes redhat-developer/quarkus-ls#334

Signed-off-by: David Thompson davthomp@redhat.com

@datho7561 datho7561 force-pushed the 334-property-expression-validation branch from 4d64288 to 5148fc9 Compare July 23, 2020 20:01
@datho7561
Copy link
Copy Markdown
Contributor Author

WIP Demo:

PropertyExpressionValidation

return n.getNodeType() == NodeType.PROPERTY;
}).map(prop -> {
return ((Property)prop).getPropertyNameWithProfile();
}).collect(Collectors.toList()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think its best if we don't do this here. If we are in an application.properties file that has no property expressions, then it seems that we didn't need to compute this in the first place.

Maybe we can:

Check in validatePropertyValueExpressions() if the property is a project property by checking MicroProfilePropertiesUtils.getProperty(propertyName, projectInfo) == null
like here:
https://github.com/eclipse/lsp4mp/blob/82c4798ac142efeb06499ece053407ef01009d46/microprofile.ls/org.eclipse.lsp4mp.ls/src/main/java/org/eclipse/lsp4mp/services/MicroProfileValidator.java#L90-L91

and if MicroProfilePropertiesUtils.getProperty(propertyName, projectInfo) == null, then we can check if it matches a property in the properties file.

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is a good idea. I'll work on getting this implemented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I switched to using lazy loading for creating the list (only create the list right before its needed, then reuse it for future tasks). The implementation is kind of messy; I am open to suggestions on how to improve it.

@datho7561 datho7561 force-pushed the 334-property-expression-validation branch 3 times, most recently from de8657a to 70fc752 Compare July 31, 2020 15:47
@datho7561 datho7561 changed the title 334 property expression validation Property expression validation Aug 11, 2020
@fbricon
Copy link
Copy Markdown
Contributor

fbricon commented Aug 19, 2020

@datho7561 please make sure new file headers follow this format:

/********************************************************************************

********************************************************************************/

@datho7561 datho7561 force-pushed the 334-property-expression-validation branch from 70fc752 to 467b838 Compare September 1, 2020 13:59
@datho7561 datho7561 marked this pull request as ready for review September 1, 2020 14:07
@datho7561 datho7561 force-pushed the 334-property-expression-validation branch from 467b838 to a9ac3fc Compare September 1, 2020 20:03
datho7561 added a commit to datho7561/vscode-microprofile that referenced this pull request Sep 1, 2020
You can now set the severity of diagnostics related to
references to unknown properties using
`microprofile.tools.validation.expression.severity`.

Requires the server-side changes in
eclipse-lsp4mp/lsp4mp#21

Signed-off-by: David Thompson <davthomp@redhat.com>
datho7561 added a commit to datho7561/vscode-microprofile that referenced this pull request Sep 1, 2020
You can now set the severity of diagnostics related to
references to unknown properties using
`microprofile.tools.validation.expression.severity`.

Requires the server-side changes in
eclipse-lsp4mp/lsp4mp#21

Signed-off-by: David Thompson <davthomp@redhat.com>
Two things are validated:
 * If there is a closing brace
 * If the referenced property exists (if it is defined in a Java file
or assigned a value in the properties file)

Closes redhat-developer/quarkus-ls#334

Signed-off-by: David Thompson <davthomp@redhat.com>
@datho7561 datho7561 force-pushed the 334-property-expression-validation branch from a9ac3fc to f923014 Compare September 2, 2020 15:05
@angelozerr angelozerr merged commit 2ae65e6 into eclipse-lsp4mp:master Sep 3, 2020
angelozerr pushed a commit to redhat-developer/vscode-microprofile that referenced this pull request Sep 3, 2020
You can now set the severity of diagnostics related to
references to unknown properties using
`microprofile.tools.validation.expression.severity`.

Requires the server-side changes in
eclipse-lsp4mp/lsp4mp#21

Signed-off-by: David Thompson <davthomp@redhat.com>
@angelozerr angelozerr added the enhancement New feature or request label Sep 11, 2020
@angelozerr angelozerr added this to the 0.1.0 milestone Sep 11, 2020
@datho7561 datho7561 deleted the 334-property-expression-validation branch July 19, 2021 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation support for property expressions

4 participants