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

Properties files with property expressions are flagged as wrong #392

Closed
jamesfalkner opened this issue Sep 8, 2021 · 11 comments
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jamesfalkner
Copy link

For example, in my application.properties I have:

%prod.quarkus.datasource.username = ${DBUSER:sa}

In vscode, I get red squigglies:

image

And the hover tooltip says:

image

Yet the app works as expected!

@angelozerr
Copy link
Contributor

angelozerr commented Sep 9, 2021

The expression validation is very basic,it checks that content of expression reference a property that you define in the properties file (ex : DBUSER:sa=XXXX). But in your case, it doesn't make sense.

The expression validation should be improved again. To remove this error, you can disable the expression validation like this:

"microprofile.tools.validation.expression.severity": "none"

or if you want to ignore validation only for quarkus.datasource.username, you can define in your settings:

"microprofile.tools.validation.expression.excluded": ["quarkus.datasource.username"]

I noticed that microprofile.tools.validation.expression.excluded is an unkwonw setting. I created a PR for that redhat-developer/vscode-microprofile#71

@AlexXuChen please review it.

@fbricon
Copy link
Collaborator

fbricon commented Sep 9, 2021

@angelozerr property expressions with a default value should not raise an error:

${expression:value} - Provides a default value after the : if the expansion doesn’t find a value.

See https://quarkus.io/guides/config-reference#property-expressions

@angelozerr
Copy link
Contributor

@angelozerr property expressions with a default value should not raise an error:

please create an issue. The original issue was about that expression property was not found.

@fbricon
Copy link
Collaborator

fbricon commented Sep 9, 2021

I disagree. Missing default property expression support is the root cause of this issue, that's why it's flagged as an error.

@angelozerr
Copy link
Contributor

angelozerr commented Sep 9, 2021

@fbricon I agree with you, default value should be supported, but even if we support it, we will have an error on DBUSER only.

I mean if you write just an expression without default value:

%prod.quarkus.application.name = ${DBUSER}

you have the same problem.

@angelozerr
Copy link
Contributor

angelozerr commented Sep 9, 2021

@jamesfalkner could you explain me where you declare the property DBUSER? In your pom.xml as inside <properties> or with https://quarkus.io/guides/datasource#named-datasource-injection ?

@jamesfalkner
Copy link
Author

jamesfalkner commented Sep 9, 2021

It's set in the environment and consumed at runtime to replace the value of ${DBUSER} with the value from the process's environment. (but it is very likely not defined in the IDE's environment - it's just assumed it'll be defined at runtime in production - hence the default value is provided as well).

@angelozerr
Copy link
Contributor

Thanks for your feedback.

Perhaps we should give the capability to define in a settings the environement variable to support correctly the diagnostic support.

@fbricon @rgrunber what do you think about that?

@rgrunber
Copy link
Member

rgrunber commented Sep 9, 2021

I suppose we could expand BuildSupport.ts to also read some setting we define for adding more properties (through system/env), but it honestly doesn't seem like this is what's required. There's plenty of ways to provide properties so I'm not sure if providing yet another source through client settings changes anything.

Should we emit a diagnostic when we encounter a variable with no default value, where we can't detect a value at build time ? The warning could mention that they should ensure it is at least defined at runtime.

@angelozerr
Copy link
Contributor

It's set in the environment

What is your configuration from https://quarkus.io/guides/config-reference#configuration-sources (System properties, Environment variables or .env)?

@angelozerr
Copy link
Contributor

Fixed with eclipse/lsp4mp#201

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants