Skip to content

Simplify a URL utility function - #1702

Merged
liblit merged 1 commit into
wala:masterfrom
liblit:simplify-UrlManipulator-relativeToAbsoluteUrl
Sep 21, 2025
Merged

liblit merged 1 commit into
wala:masterfrom
liblit:simplify-UrlManipulator-relativeToAbsoluteUrl

Conversation

@liblit

@liblit liblit commented Sep 21, 2025

Copy link
Copy Markdown
Contributor

Personally, I'm skeptical of the nonstandard normalizations that relativeToAbsoluteUrl initially applies to urlFound: replacing \ with / and converting to lower case. But that's what was done before, presumably for good reason.

Note that the new implementation of relativeToAbsoluteUrl differs from the old implementation in how it handles .. in the interior of the urlFound path. The new implementation normalizes those away, whereas the old implementation did not. I'm OK with this change, so I've updated the test suite accordingly.

Personally, I'm skeptical of the nonstandard normalizations that
`relativeToAbsoluteUrl` initially applies to `urlFound`:  replacing
`\` with `/` and converting to lower case.  But that's what was done
before, presumably for good reason.

Note that the new implementation of `relativeToAbsoluteUrl` differs
from the old implementation in how it handles `..` in the interior of
the `urlFound` path.  The new implementation normalizes those away,
whereas the old implementation did not.  I'm OK with this change, so
I've updated the test suite accordingly.
@liblit
liblit requested a review from msridhar September 21, 2025 18:02
@liblit liblit self-assigned this Sep 21, 2025
@liblit liblit added the cleanup API cleanup and refactoring label Sep 21, 2025
@liblit
liblit enabled auto-merge September 21, 2025 18:02
@codecov

codecov Bot commented Sep 21, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.22%. Comparing base (224c541) to head (0e2bfcc).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...java/com/ibm/wala/cast/js/html/UrlManipulator.java 33.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1702      +/-   ##
============================================
- Coverage     50.23%   50.22%   -0.02%     
+ Complexity    12659    12651       -8     
============================================
  Files          1365     1365              
  Lines         85220    85191      -29     
  Branches      14733    14726       -7     
============================================
- Hits          42814    42785      -29     
  Misses        37608    37608              
  Partials       4798     4798              

☔ View full report in Codecov by Sentry.
📢 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.

@github-actions

Copy link
Copy Markdown

Test Results

  719 files  ±0    719 suites  ±0   4h 47m 10s ⏱️ - 4m 5s
  789 tests ±0    770 ✅ ±0   19 💤 ±0  0 ❌ ±0 
4 618 runs  ±0  4 502 ✅ ±0  116 💤 ±0  0 ❌ ±0 

Results for commit 0e2bfcc. ± Comparison against base commit 224c541.

@liblit
liblit added this pull request to the merge queue Sep 21, 2025
Merged via the queue into wala:master with commit 5fc1296 Sep 21, 2025
11 of 12 checks passed
@liblit
liblit deleted the simplify-UrlManipulator-relativeToAbsoluteUrl branch September 21, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup API cleanup and refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants