-
Notifications
You must be signed in to change notification settings - Fork 2
Lara-dsl-deprecation Checkpoint 5 #23
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
base: lara-dsl-deprecation-checkpoint4
Are you sure you want to change the base?
Conversation
Fix null handling and improve error messages in ClassMap, ClassSet, FunctionClassMap, and MultiFunction
Implement null supplier validation in Lazy and LazyString classes
Implement null checks and improve error handling in XML classes
Fix null input handling and improve validation in RegisterUtils methods
Refactor and enhance utility classes with improved validation and error handling - AverageType.java: Added checks for null and empty collections, improved handling for geometric mean and harmonic mean calculations, and introduced a safe method for large datasets. - BufferedStringBuilder.java: Implemented validation for null output files and added null handling in append method. - BuilderWithIndentation.java: Added validation for null tab strings and strings in add and addLines methods. - CachedItems.java: Enhanced thread safety with atomic counters for cache hits and misses, and validated mapper function in constructor. - ClassMapper.java: Added validation for null classes in add and map methods. - JarPath.java: Improved error handling for invalid paths with logging. - LineStream.java: Added null check before storing lines. - NullStringBuilder.java: Updated constructor to avoid validation for null file. - PersistenceFormat.java: Added validation for null file and class parameters in read and write methods. - ScheduledLinesBuilder.java: Fixed maxLevel calculation to ensure all levels are shown. - StringList.java: Updated split method to preserve empty strings. - HeapBar.java: Improved event handling for timer cancellation. - MemProgressBarUpdater.java: Added validation for null JProgressBar in constructor. - AverageTypeTest.java: Updated tests to reflect changes in AverageType behavior and fixed assertions. - BufferedStringBuilderTest.java: Updated tests to check for proper exception handling and null handling. - BuilderWithIndentationTest.java: Updated tests to validate null inputs and expected behavior for empty strings. - CachedItemsTest.java: Enhanced tests for thread safety and race conditions. - ClassMapperTest.java: Updated tests to validate null inputs and interface hierarchy support. - PersistenceFormatTest.java: Updated tests to validate null file and class parameters. - ScheduledLinesBuilderTest.java: Updated tests to validate correct output for scheduled lines. - StringListTest.java: Updated tests to validate correct handling of split behavior. - MemProgressBarUpdaterTest.java: Updated tests to validate exception handling for null JProgressBar.
Enhance null handling across provider framework - Updated CachedStringProvider to use Optional.ofNullable for null values. - Modified GenericFileResourceProvider to validate target folder and throw IllegalArgumentException for null. - Added null checks in StringProvider factory methods and Resources constructor. - Updated tests to ensure null values are handled correctly and exceptions are thrown as expected.
Enhance MapModel to validate indices and support row-wise updates
Refactor tests to improve exception handling and assertions - Updated InputModeTest to validate null extensions and folder levels, ensuring IllegalArgumentException is thrown with clear messages instead of NullPointerException. - Modified JobProgressTest to ensure that exceeding job counts handles gracefully without throwing exceptions. - Adjusted JobTest to confirm that the interrupted flag is properly propagated when execution throws an exception, returning 0 instead of -1. - Revised JobUtilsTest to reflect the correct behavior of job execution and interruption handling, ensuring jobs with exceptions stop execution as expected and that empty extensions return an empty list.
…ions to indicate that certain issues are not bugs, but rather expected behaviors that require test adjustments.
Fix null handling in ParserResult and update StringParser trim behavior assertions
Enhance timeout handling in ChannelConsumer: handle negative timeouts as zero and cap extreme values to prevent overflow. Update tests to validate these behaviors.
Refactor OutputType enum and ProcessOutputAsString class for correct output handling and null management
Fix range calculation and handle empty header in CsvWriter
…racking and add clearCache method for test isolation. Update XmlNode to return text content from document element. Adjust PrintOnceTest for new cache clearing method and improve concurrent message verification. Fix XmlDocumentTest comment regarding valid XML.
… bugs in the process.
- Replaced instances of SpecsCheck.checkNotNull with Objects.requireNonNull in various classes to standardize null checks. - Updated methods in AsmFieldData, DeployUtils, EsprimaComment, EsprimaNode, SpecsCollections, SpecsIo, SpecsStrings, and several others. - Improved code readability and consistency by utilizing Java's built-in null-checking mechanism.
…opposed to tabs. Also checked for commented code.
…ull elements and improve safety
…ot of listeners and logging exceptions without propagation; update tests to demonstrate unsafe notifier behavior with null events.
…nterface for consistency and improved error handling
… readLine() and handle null reader input safely; update StreamCatcherTest to validate output length accurately.
…PointerException for null parameters instead of IllegalArgumentException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR focuses on updating test files to reflect fixes in the production code, primarily replacing assertions that documented bugs with assertions that verify correct behavior. The changes address previously documented issues related to null handling, validation, thread safety, and interface hierarchy support.
Key Changes:
- Updated test assertions to expect correct behavior instead of buggy behavior
- Removed "BUG" comments and updated corresponding assertions
- Enhanced thread safety in test utilities (CopyOnWriteArrayList)
- Fixed test expectations for null handling and validation
Reviewed Changes
Copilot reviewed 297 out of 507 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TextAreaHandlerTest.java | Updated exception handling test to capture throwables on EDT without uncaught exceptions |
| LoggingOutputStreamTest.java | Changed to thread-safe CopyOnWriteArrayList for concurrent logging tests |
| LogSourceInfoTest.java | Refactored setup/teardown to use factory defaults instead of copying original state |
| ThreadSafeLazyTest.java | Updated null supplier test to expect NullPointerException |
| LazyTest.java | Updated null supplier tests to expect NullPointerException with proper messages |
| LazyStringTest.java | Fixed null value handling test to expect "null" string instead of null |
| JobUtilsTest.java | Updated interrupted job tests to expect correct return values and behavior |
| JobTest.java | Fixed exception handling test to expect correct interrupted flag propagation |
| JobProgressTest.java | Updated nextMessage tests to handle gracefully instead of throwing exceptions |
| InputModeTest.java | Updated null validation tests to expect IllegalArgumentException with messages |
| ResourceCollectionTest.java | Changed getter method calls from getId()/getResources() to id()/resources() |
| InputFilesTest.java | Changed field access from direct field access to accessor methods |
| GraphSerializableTest.java | Changed field access to accessor methods for serializable fields |
| EventNotifierTest.java | Updated unsafe notifier implementation to demonstrate null rejection |
| ActionsMapTest.java | Updated null action tests to expect NullPointerException |
| CsvWriterTest.java | Fixed formula range calculation tests and empty initialization handling |
| PushingQueueTest.java | Updated toString test to include null in expected output |
| ChannelConsumerTest.java | Re-enabled timeout edge cases test |
| MultiFunctionTest.java | Updated default value tests to expect correct behavior and fluent interface |
| FunctionClassMapTest.java | Fixed null default function return handling |
| ClassSetTest.java | Updated null handling and interface hierarchy tests |
| ClassMapTest.java | Fixed null value handling and null class validation |
| RegisterUtilsTest.java | Updated null input handling and multiple underscore parsing |
| JumpDetectorTest.java | Added logic to distinguish between _TAKEN and _NOT_TAKEN instructions |
| DelaySlotBranchCorrectorTest.java | Updated jump handling tests and added nested jump test |
| ArithmeticResult32Test.java | Changed field access to accessor methods |
| SpecsSystemTest.java | Removed duplicate test methods and changed field visibility |
| SpecsSwingTest.java | Added headless environment guards and mocking for file browser tests |
| SpecsNumbersTest.java | Updated null input test to expect NullPointerException |
| SpecsCheckTest.java | Added @SuppressWarnings for deprecated checkNotNull usage |
| SpecsAsmTest.java | Changed field access to accessor methods |
| XmlNodes.java | Added null checks in factory and utility methods |
| XmlNode.java | Enhanced getText/setText with null handling and empty text support |
| XmlGenericNode.java | Added null validation in constructor |
| XmlElement.java | Added null validation for attribute operations |
| XmlDocument.java | Added null validation and error handling for factory methods |
| MemProgressBarUpdater.java | Added null check and EDT-safe initialization |
| HeapWindow.java | Added headless environment support |
| HeapBar.java | Added null checks for timer cancellation |
| Table.java | Replaced manual map initialization with computeIfAbsent |
| StringSlice.java | Applied pattern matching for instanceof |
| StringList.java | Simplified equals method and improved decode/encode |
| StringLines.java | Replaced .length() > 0 with !isEmpty() |
| SpecsThreadLocal.java | Replaced Preconditions with Objects.requireNonNull |
| ScheduledLinesBuilder.java | Fixed toString for empty scheduledLines |
| ProgressCounter.java | Added validation, documentation, and default constructor |
| PrintOnce.java | Changed to thread-safe ConcurrentHashMap.newKeySet() |
| PersistenceFormat.java | Added null validation for file and class parameters |
| PatternDetector.java | Added null-safe equals check |
| NullStringBuilder.java | Added toString override |
| MemoryProfiler.java | Added lifecycle management and proper thread control |
| LineStream.java | Removed unnecessary try-catch, added null check for lastLines |
| LastUsedItems.java | Added null-safe equals and Optional.ofNullable |
| JarPath.java | Added exception handling for invalid paths |
| ClassMapper.java | Added null validation and recursive interface mapping |
| CachedValue.java | Enhanced thread safety with volatile and synchronization |
| CachedItems.java | Improved thread safety with atomic counters and computeIfAbsent |
| BuilderWithIndentation.java | Added null validation and optimized indentation |
| BufferedStringBuilder.java | Added null validation, toString, and persisted content cache |
| AverageType.java | Enhanced geometric mean calculation with overflow protection |
| OutputType.java | Swapped enum order to match conventional stdout/stderr ordering |
| DebugBufferedReader.java | Added null handling and improved debug output |
| ProcessOutputAsString.java | Fixed null value handling in getOutput |
| ProcessOutput.java | Replaced string concatenation with StringBuilder.append |
| StreamToString.java | Replaced System.getProperty with System.lineSeparator |
| StreamCatcher.java | Replaced System.getProperty with System.lineSeparator |
| MapModelV2.java | Added @serial annotations |
| MapModel.java | Added bounds validation and improved setValueAt |
| GenericActionListener.java | Added @serial annotation |
| StringSplitterRules.java | Changed method calls to use record accessor methods |
| StringSplitter.java | Changed method calls to use record accessor methods |
| StringSliceWithSplit.java | Changed method calls to use record accessor methods |
| SplitResult.java | Converted class to record |
| StringParsersLegacy.java | Changed method calls to use record accessor methods |
| StringParsers.java | Changed Arrays.asList to Collections.singletonList |
| StringParser.java | Fixed trim detection logic to use object comparison |
| ParserResult.java | Converted class to record with Optional.ofNullable |
| ReporterUtils.java | Replaced Preconditions with Objects.requireNonNull |
| Reporter.java | Added synchronization to default methods |
| GenericWebResourceProvider.java | Converted class to record |
| GenericResource.java | Converted class to record |
| GenericFileResourceProvider.java | Added null validation and fixed isVersioned logic |
| CachedStringProvider.java | Improved Optional handling |
| WebResourceProvider.java | Changed method names to record accessor style |
| StringProvider.java | Added null validation |
| Resources.java | Added null validation |
| ResourceProvider.java | Simplified list operations with Collections.addAll |
| ProvidersSupport.java | Replaced Preconditions with Objects.requireNonNull |
| FileResourceProvider.java | Changed method name and improved null handling |
| FileResourceManager.java | Improved Optional handling |
| SpecsProperty.java | Added headless check for heap window |
| SpecsProperties.java | Improved property access with containsKey |
| CommentParser.java | Improved Optional handling |
| PragmaRule.java | Simplified string joining |
| PragmaMacroRule.java | Simplified string joining |
| MultiLineCommentRule.java | Simplified string joining |
| GenericTextElement.java | Converted class to record |
| ArgumentsParser.java | Replaced Arrays.asList with List.of |
| LineParser.java | Replaced .length() == 0 with !isEmpty() |
| GenericCodec.java | Added @serial annotation |
| TreeNode.java | Replaced Preconditions with Objects.requireNonNull |
| ATreeNode.java | Replaced SpecsCheck/Preconditions with Objects.requireNonNull |
| settings.gradle | Fixed includeBuild path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static Integer decodeFlagBit(String registerFlagName) { | ||
| // Handle null input gracefully | ||
| if (registerFlagName == null) { | ||
| SpecsLogs.getLogger().warning("Cannot decode flag bit from null input"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SpecsLogs.warn()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were 81 instances of SpecsLogs.getLogger().warning(...). I assumed you wanted all of them changed and so I did so.
SpecsUtils/src/pt/up/fe/specs/util/enums/EnumHelperWithValue.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| return new GenericFileResourceProvider(existingFile, version, false); | ||
| return new GenericFileResourceProvider(existingFile, version, version != null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is equivalent, the isVersioned flag IIRC is related to the name of the resource (i.e., if it is expected that the version appears in the name). This is different from providing a version, which will be used to check if a new version should be used (regardless if the version is supposed to be in the name or not)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the WebResourceProvider force the version name to be in the file name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking closely at the code, this variable might not even be needed. It is only used in one place, inside GenericFileResourceProvider:
@Override
public FileResourceProvider createResourceVersion(String version) {
if (isVersioned) {
throw new NotImplementedException("Not implemented when file is versioned");
}
// Create new versioned file
return newInstance(existingFile, version);
}What the change in the code is doing is preventing a versioned file to change version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changed was introduced as a fix to:
Bug 7: GenericFileResourceProvider Always Sets isVersioned to False
Location: pt.up.fe.specs.util.providers.impl.GenericFileResourceProvider.newInstance() method
Issue: The newInstance method always passes false for the isVersioned parameter regardless of whether a version is provided. This means the createResourceVersion method never throws NotImplementedException even for versioned providers.
Impact: Versioned providers can have their versions changed when they shouldn't be able to, violating the intended behavior documented in the createResourceVersion method.
Current Behavior: All providers are marked as non-versioned, allowing version changes on any provider.
Expected Behavior: Providers created with a version should be marked as versioned and should throw NotImplementedException when createResourceVersion is called.
Recommendation: Fix the newInstance method to set isVersioned to true when a version is provided.
…eadability and performance
…ed the Z3Helper project as it was deprecated. All code was deleted because it was unused or deprecated.
- Updated multiple instances across various classes to replace SpecsLogs.getLogger().warning with SpecsLogs.warn for improved readability and consistency in logging practices. - This change enhances the clarity of log messages throughout the codebase, ensuring a uniform approach to logging warnings.
…ode' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
NOT TO BE MERGED. FOR REVIEW PURPOSES ONLY. THIS IS PART OF A STACKED DIFF.