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

Importing 2024 project with beta-2 produces incorrect import lines for some units-related modules #51

Open
chauser opened this issue Nov 25, 2024 · 7 comments · Fixed by wpilibsuite/vscode-wpilib#726
Labels
bug Something isn't working Pending The issue will be fixed in the next release

Comments

@chauser
Copy link

chauser commented Nov 25, 2024

Describe the bug
After importing our 2024 robot code project with beta 2 there are incorrect imports:
import static edu.wpi.first.units.measure.Units.*; -- should be edu.wpi.first.units.Units.* -- and
import edu.wpi.first.units.measure.Measure; -- should be edu.wpi.first.units.Measure
It appears this is a matter of overzealous conversion rather than missed conversion as the correct imports are what was in the original code.

To Reproduce
Steps to reproduce the behavior:
Import a Java project with files containing import static edu.wpi.first.units.Units.*; and/or import edu.wpi.first.units.Measure;

  • Link to code (if applicable):

Expected behavior
Those lines are unchanged.

Desktop (please complete the following information if applicable):

  • OS: Linux (Ubuntu 22.04)
  • OS Language: English
  • Project Information: WPILib Information:
    Project Version: 2025.1.1-beta-2
    VS Code Version: 1.94.2
    WPILib Extension Version: 2025.1.1-beta-2
    C++ Extension Version: 1.22.9
    Java Extension Version: 1.36.2024092708
    Java Debug Extension Version: 0.58.2024090204
    Java Dependencies Extension Version 0.24.0
    Java Version: 17
    Java Location: /home/hauser/wpilib/2025/jdk
    Vendor Libraries:
    PathplannerLib (2025.0.0-beta-4)
    CTRE-Phoenix (v5) (5.34.0-beta-2)
    CTRE-Phoenix (v6) (25.0.0-beta-3)
    REVLib (2025.0.0-beta-2)
    Studica (2025.1.1-beta-3)
    WPILib-New-Commands (1.0.0)
    photonlib (v2025.0.0-beta-5)

roboRIO (please complete the following information if applicable):

  • roboRIO 1 or 2?: N/A

Additional context
I don't know for certain that what I think are the correct imports really are correct -- there are still errors in the code but I think, based on the javadocs, that what I've said above is right. Certainly, the generated import lines do not work.

@chauser chauser added the bug Something isn't working label Nov 25, 2024
@sciencewhiz
Copy link
Contributor

sciencewhiz commented Nov 25, 2024

Can you post a link to the 2024 code, for testing purposes?

I'm not sure changing edu.wpi.first.units.measure.Measure to edu.wpi.first.units.Measure should be necessary, vs deleting it.

The importer didn't do this one because it was capturing words, so the * didn't trigger it.: import static edu.wpi.first.units.measure.Units.; -- should be edu.wpi.first.units.Units.

@chauser
Copy link
Author

chauser commented Nov 25, 2024

Here's a link to the repo -- I made it public. The main branch exhibits the problem when imported. https://bitbucket.org/sciborgs4061/robot-2024/

@agasser
Copy link

agasser commented Nov 25, 2024

The MutableMeasure import is also incorrectly converted from import edu.wpi.first.units.MutableMeasure; to import edu.wpi.first.units.measure.MutableMeasure;

The conversion also didn't properly convert mutable measurement types. Although I agree the classes should be called MutableDistance, etc, they are called MutDistance etc.

Image

sciencewhiz added a commit to sciencewhiz/vscode-wpilib that referenced this issue Nov 28, 2024
Fixes Mutable imports and units
Fixes wildcard import for Units
Converts MutableMeasure.zero
Fixes wpilibsuite/2025Beta#51
sciencewhiz added a commit to sciencewhiz/vscode-wpilib that referenced this issue Nov 28, 2024
Fixes Mutable imports and units
Fixes wildcard import for Units
Converts MutableMeasure.zero
Fixes wpilibsuite/2025Beta#51
sciencewhiz added a commit to sciencewhiz/vscode-wpilib that referenced this issue Nov 28, 2024
Fixes Mutable imports and units
Fixes wildcard import for Units
Converts MutableMeasure.zero
Fixes wpilibsuite/2025Beta#51
@sciencewhiz sciencewhiz reopened this Dec 13, 2024
@sciencewhiz sciencewhiz added the Pending The issue will be fixed in the next release label Dec 13, 2024
@sciencewhiz
Copy link
Contributor

Please test again with beta 3. There were several more improvements.

@chauser
Copy link
Author

chauser commented Dec 18, 2024

Much better. There are still some missing imports after the automatic conversion. For example, in Constants.java line 93 Measure<Velocity<Distance>> was converted to LinearVelocity but the necessary import wasn't added. The same thing happened for angular velocity and acceleration and linear acceleration. It isn't a big deal to add the imports manually but if it can be added to the import process it would be nice.

The remaining units-related errors after import have to do with more complicated stuff we were attempting with the old units system last year -- I'm not surprised that those aren't automated.

Thanks for the work to improve this.

@sciencewhiz
Copy link
Contributor

For example, in Constants.java line 93 Measure<Velocity<Distance>> was converted to LinearVelocity but the necessary import wasn't added. The same thing happened for angular velocity and acceleration and linear acceleration. It isn't a big deal to add the imports manually but if it can be added to the import process it would be nice.

That's not possible without significant changes to the import process. Right now it just replaces with regex. It would have get a lot smarter to find something in one place and make a change somewhere else.

@chauser
Copy link
Author

chauser commented Dec 19, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Pending The issue will be fixed in the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants