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

Allow type transformations with mapper for Map #1902

Merged
merged 1 commit into from
Dec 29, 2023

Conversation

weirdwater
Copy link
Contributor

@weirdwater weirdwater commented Dec 29, 2023

Hi, while aggregating results using Map's mapper I noticed that it does not allow the key and value types to change. Its return type matches the input types of the map functions F and G, rather than their return types. This restricts the map functions to only transform the values within the same type.

The proposed change updates the return type of mapper to match that of the given map functions. Existing usage should not be impacted by this change as the current return types of F and G are inferred.

The mapper functions for Set and List already use matching return types. As far as I could see those are the only other instances of mapper.

Running `mvn test` yields the same result before and after the change.
[ERROR] Tests run: 5898, Failures: 0, Errors: 3, Skipped: 201, Time elapsed: 73.458 s <<< FAILURE! - in org.rascalmpl.test.AllSuiteParallel
[ERROR] hamcrestJarM3RemainedTheSame: <4276,168>(org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester)  Time elapsed: 0.005 s  <<< ERROR!
org.rascalmpl.exceptions.Throw: /Users/arjobruijnes/Documents/Projects/rascal/src/org/rascalmpl/library/lang/rascal/tests/library/lang/java/m3/BasicM3Tests.rsc:172,13: "typeDependency has different value"
	at org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester.runTests(RascalJUnitParallelRecursiveTestRunner.java:226)
	at org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester.run(RascalJUnitParallelRecursiveTestRunner.java:207)

[ERROR] junitM3RemainedTheSame: <2582,183>(org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester) Time elapsed: 0.001 s <<< ERROR!
org.rascalmpl.exceptions.Throw: /Users/arjobruijnes/Documents/Projects/rascal/src/org/rascalmpl/library/lang/rascal/tests/library/lang/java/m3/BasicM3Tests.rsc:172,13: "messages has different value"
at org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester.runTests(RascalJUnitParallelRecursiveTestRunner.java:226)
at org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester.run(RascalJUnitParallelRecursiveTestRunner.java:207)

[ERROR] junitASTsRemainedTheSame: <3250,189>(org.rascalmpl.test.infrastructure.RascalJUnitParallelRecursiveTestRunner$ModuleTester) Time elapsed: 0 s <<< ERROR!
java.lang.Exception:
Test lang::rascal::tests::library::lang::java::m3::BasicM3Tests::junitASTsRemainedTheSame failed due to
test returned false

[INFO] Running org.rascalmpl.TypeReificationTest
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.rascalmpl.TypeReificationTest
[INFO] Running org.rascalmpl.MatchFingerprintTest
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.rascalmpl.MatchFingerprintTest
[INFO]
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] RascalJUnitParallelRecursiveTestRunner$ModuleTester.run:207->runTests:226 » Throw
[ERROR] RascalJUnitParallelRecursiveTestRunner$ModuleTester.junitASTsRemainedTheSame: <3250,189> »
[ERROR] RascalJUnitParallelRecursiveTestRunner$ModuleTester.run:207->runTests:226 » Throw
[INFO]
[ERROR] Tests run: 5911, Failures: 0, Errors: 3, Skipped: 201
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 10:14 min
[INFO] Finished at: 2023-12-29T00:56:01+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.2:test (default-test) on project rascal: There are test failures.
[ERROR]
[ERROR] Please refer to /Users/arjobruijnes/Documents/Projects/rascal/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

Matching the return type of the mapper function with the return types of the given map functions F and G, allows the mapper function to be used to transform key and value types.

Previous definition only works for map functions that return the same type as they are given.
@jurgenvinju
Copy link
Member

jurgenvinju commented Dec 29, 2023

Thanks @weirdwater ! To let the tests go right pls compile with a JDK11. But that will be done by the action as well.

@jurgenvinju
Copy link
Member

I think we didn't catch this one earlier because people use map comprehensions a lot. Still, amazing how long this bug has lived there. 🤓

Copy link

codecov bot commented Dec 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f604f30) 49% compared to head (c714bf6) 49%.

Additional details and impacted files
@@           Coverage Diff           @@
##              main   #1902   +/-   ##
=======================================
- Coverage       49%     49%   -1%     
+ Complexity    6159    6158    -1     
=======================================
  Files          661     661           
  Lines        58698   58698           
  Branches      8547    8547           
=======================================
- Hits         28937   28933    -4     
  Misses       27565   27565           
- Partials      2196    2200    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jurgenvinju jurgenvinju merged commit 7338b14 into usethesource:main Dec 29, 2023
8 checks passed
@weirdwater
Copy link
Contributor Author

Thanks for merging. I am still getting used to the language and had not thought of trying comprehension on Maps yet, neat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants