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

Error messages like "Reference to undefined variable foo" should explain where the variable is referenced #2591

Closed
j256 opened this issue Dec 5, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@j256
Copy link

j256 commented Dec 5, 2024

When running a junit test or other runtime configuration, I was getting an undefined variable error but I had no idea what it was talking about. Finally I found that the VM arguments had a ${foo} that did not have a value that was copied there by maven's surefire plugin when I upgraded to 2024.09. The message would be improved if it was something like:

  Reference to undefined variable 'foo' in Runtime Configuration VM arguments.

I did a naive walk thru the Eclipse code looking for the error message but don't have enough domain knowledge to recommend a fix. If someone points me to the line generating that code I will work to add that context.

@j256 j256 changed the title Error messages like "Reference to undefined variable foo" should explain where the variable is referenced Error messages like "Reference to undefined variable foo" should explain where the variable is being used Dec 5, 2024
@j256 j256 changed the title Error messages like "Reference to undefined variable foo" should explain where the variable is being used Error messages like "Reference to undefined variable foo" should explain where the variable is referenced Dec 5, 2024
@jukzi jukzi added the enhancement New feature or request label Dec 6, 2024
@jukzi
Copy link
Contributor

jukzi commented Dec 6, 2024

full text search of "Reference to undefined variable" leads to VariablesMessages.properties StringSubstitutionEngine_3
full text search of "StringSubstitutionEngine_3" leads to org.eclipse.core.internal.variables.StringSubstitutionEngine.resolve(VariableReference, boolean, boolean, IStringVariableManager) line 263

@merks
Copy link
Contributor

merks commented Dec 6, 2024

I ran into this with a client project too. Normally one authors a launch configuration and one knows if one has decided to use string substitutions in it, so then the message is obvious when it appears. But now it seems m2e is the one adding such variables (if the pom.xml uses a variable); perhaps m2e should be expanding the variable, and if not, perhaps should generate a string substitution for it. E.g., there was stuff like this in the pom.xml so for the Oomph setup I had to define an empty string substitution for jacoco.argLine:

          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-surefire-plugin</artifactId>
            <configuration>
              <argLine>${jacoco.argLine}</argLine>
            </configuration>
          </plugin

@laeubi @HannesWell

Has something changes in m2e such that variables in the pom end up being variables in the launch configuration, e.g., when the variable can't be expanded by maven/m2e? Is that a bug or a feature? It's really confusing for users who have might never have used a string substitution...

@j256

I assume this is similar to your case? I believe it's m2e doing that copying not anything in Maven itself. What help would you expect from the message? A description of which tab of which launch configuration to inspect? Often users aren't really all that aware of the launch configuration either...

@HannesWell
Copy link
Member

Has something changes in m2e such that variables in the pom end up being variables in the launch configuration, e.g., when the variable can't be expanded by maven/m2e? Is that a bug or a feature?
It's a new feature in m2e that more elements of the surefire configuration are automatically transferred into a JUnit test launch configuration. But that feature needed some fine-tuning.
This issue sounds very much like:

This should be fixed in the latest m2e release that's also included in the 2024-12 SimRel.
Can you please try that?

If the error still exists, please create an issue in:
https://github.com/eclipse-m2e/m2e-core

Closing this here because it's a bug in m2e, that is hopefully fixed already.

@j256
Copy link
Author

j256 commented Dec 8, 2024

I assume this is similar to your case? I believe it's m2e doing that copying not anything in Maven itself. What help would you expect from the message? A description of which tab of which launch configuration to inspect? Often users aren't really all that aware of the launch configuration either...

Yes this was a problem with m2e that I hear was fixed. However, I still think that if Eclipse has a variable that isn't resolved it should, yes, describe the location to inspect to find the variable. In this case, I would assume the current launch configuration so just the location would be enough. In my OP the example just appended the location "in Runtime Configuration VM arguments":

Reference to undefined variable 'foo' in Runtime Configuration VM arguments.

@merks
Copy link
Contributor

merks commented Dec 9, 2024

Don't take this the wrong way. It's highly unlikely anyone will implement this because.

  • Everyone is busy on something else which is mostly their own personal priorities or those of the people who pay them.
  • It's not an easy problem to solve because the place that throws the exception has no idea where the variable comes from so the callers need to do the processing. If you look at those callers, it's a large tree so it's not even a "good first issue" to delegate to someone interested in making a first contribution:
    image

So of course it would be nice to have better information, but someone will need to do that work; someone who has the time, the skills, and considers this specific issue their top priority for the investment of their time and skills. Yes of course we could dismiss many issues in the same way for the same reason and folks are likely to take offense, and who could blame them? But when you look at the current 398 open issues, being realistic, it might be better if more of them were closed because just triaging and managing 398 issues (which is something else rather neglected) is a lot of work... 😞

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

No branches or pull requests

4 participants