Skip to content

Distinguish between cases "resolver doesn't exist" and "resolver does exist, but fails" when resolving logical locations - #2901

Merged
DavyLandman merged 18 commits into
mainfrom
fix/1193-tryresolve-instead-of-saferesolve
Oct 1, 2026
Merged

DavyLandman merged 18 commits into
mainfrom
fix/1193-tryresolve-instead-of-saferesolve

Conversation

@sungshik

@sungshik sungshik commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

The new `tryResolve` is the same as the existing `safeResolve`, except `tryResolve` propagates both `IOException`s and unchecked exceptions to the caller, whereas `safeResolve` catches and ignores all exceptions.
…e *already* allows `IOException`s to be thrown (i.e., callers of those methods should already be sufficiently resilient)
@sungshik
sungshik force-pushed the fix/1193-tryresolve-instead-of-saferesolve branch from d5a8856 to 046db25 Compare September 28, 2026 12:40
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java
@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.34043% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 45%. Comparing base (d18507a) to head (b7a9954).

Files with missing lines Patch % Lines
src/org/rascalmpl/uri/URIResolverRegistry.java 71% 9 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              main   #2901   +/-   ##
=======================================
  Coverage       45%     45%           
- Complexity    6799    6811   +12     
=======================================
  Files          843     844    +1     
  Lines        68828   68824    -4     
  Branches     10030   10028    -2     
=======================================
+ Hits         31385   31420   +35     
+ Misses       35054   35012   -42     
- Partials      2389    2392    +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sungshik
sungshik marked this pull request as ready for review September 28, 2026 13:00

@DavyLandman DavyLandman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this PR is doing too little.

It should also be dealing with the case of a resolver that is not specific to an authority, but deal with case where it says to resolve all kinds of authorities, but sometimes fail.

The original issue with mkDirectory(|project://not-existing|) is a nice example of this. (but I understand that there are 2 project resolvers at play, and they make debugging/developing this feature messy)

Comment thread src/org/rascalmpl/uri/UnsupportedSchemeException.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated

@DavyLandman DavyLandman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, some small semantic/perf changes I propose.

Comment thread src/org/rascalmpl/library/lang/rascal/tests/library/IO.rsc Outdated
Comment thread src/org/rascalmpl/library/lang/rascal/tests/library/IO.rsc Outdated
Comment thread src/org/rascalmpl/uri/ILogicalSourceLocationResolver.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated
Comment thread src/org/rascalmpl/uri/URIResolverRegistry.java Outdated
Comment thread src/org/rascalmpl/uri/ILogicalSourceLocationResolver.java Outdated
@sonarqubecloud

Copy link
Copy Markdown

@DavyLandman
DavyLandman merged commit 3f7f467 into main Oct 1, 2026
9 checks passed
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.

mkDirectory on a project URI fails with confusing message

2 participants