Fix location tracking and remove static caching to prevent parallel test failures#24
Open
Fix location tracking and remove static caching to prevent parallel test failures#24
Conversation
Changes: - Added optional location field to Modifier case class - Added optional location field to Annotation case class - Updated tokenToModifier() to preserve token locations - Updated parseModifiersAndAnnotations() to create spanned locations for compound modifiers (e.g., 'with sharing') - Updated parseAnnotation() to capture full annotation span from @ through parameters Both location fields default to None for backward compatibility and exclude location from equality/hash checks to preserve existing semantics. All 6,302 tests pass with these changes. Resolves #19
nawforce
force-pushed
the
add-location-tracking-to-modifier-and-annotation
branch
from
01ad5b8 to
8efc3ca
Compare
…est failures This commit completes issue #19 by implementing proper location tracking for Modifier and Annotation types and fixing several related issues: **Location Tracking:** - Changed lineOffset from 1-based to 0-based indexing to match ANTLR conventions - Made end positions exclusive ([start, end) half-open interval) for both columns and byte offsets - Enabled location validation in tests by comparing ANTLR vs outline parser locations **Caching Bug Fix:** - Removed intern() methods and static caches from Annotation and Modifier companion objects - Removed all intern() calls from factory methods in Types.scala - The static caches were causing parallel test failures because ArrayInternCache uses equality that ignores location field, returning cached instances with wrong locations **Test Infrastructure:** - Added syncNodeModules task to ensure node_modules are available for ScalaJS tests - Optimized ANTLR location extraction by skipping byte offset calculation (too expensive) - Updated Compare.scala to ignore byte offsets when comparing locations - Added UndefOr[Token] handling for ScalaJS where context.stop might be undefined **Dependency Updates:** - Upgraded apex-ls to 6.0.2 to fix lexer errors with malformed files - Upgraded Scala.js to 1.18.2 for compatibility with apex-ls 6.0.2 All 6,303 tests pass successfully with location tracking fully enabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
nawforce
force-pushed
the
add-location-tracking-to-modifier-and-annotation
branch
from
8efc3ca to
86530c7
Compare
| @@ -1,4 +1,4 @@ | |||
| addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.12.0") | |||
| addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.18.2") | |||
| addSbtPlugin("org.portable-scala" % "sbt-scalajs-crossproject" % "1.2.0") | |||
Contributor
There was a problem hiding this comment.
Should we update the other dependencies here
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19
Summary
This PR implements proper location tracking for Modifier and Annotation types and fixes parallel test failures caused by static caching.
Changes
Location Tracking
lineOffsetfrom 1-based to 0-based indexing to match ANTLR conventions[start, end)half-open interval) for both columns and byte offsetsCaching Bug Fix
intern()methods and static caches fromAnnotationandModifiercompanion objectsintern()calls from factory methods inTypes.scalaArrayInternCacheuses equality that ignores location field, returning cached instances with wrong locationsTest Infrastructure
syncNodeModulestask to ensure node_modules are available for ScalaJS testsCompare.scalato ignore byte offsets when comparing locationsUndefOr[Token]handling for ScalaJS wherecontext.stopmight be undefinedDependency Updates
Testing
All 6,303 tests pass successfully with location tracking fully enabled.
Related Issues