Skip to content

Conversation

@n0nag0n
Copy link
Collaborator

@n0nag0n n0nag0n commented Jul 20, 2025

No description provided.

This comment was marked as outdated.

@n0nag0n n0nag0n requested a review from Copilot July 20, 2025 15:50
Copy link
Contributor

Copilot AI left a 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 introduces a new JSON utility class to centralize JSON handling across the FlightPHP framework and addresses PHP Code Sniffer warnings by adding ignore comments for heredoc/nowdoc syntax issues.

  • Adds a comprehensive Json utility class with encoding, decoding, validation, and error handling methods
  • Updates the Engine class to use the new Json utility instead of direct json_encode calls
  • Adds phpcs:ignore comments to suppress code style warnings for heredoc/nowdoc syntax

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
flight/util/Json.php New utility class providing centralized JSON operations with consistent error handling
flight/Engine.php Updated to use Json utility class and added phpcs:ignore comment
tests/JsonTest.php Comprehensive test suite for the new Json utility class
tests/EngineTest.php Updated tests to handle new Json utility behavior
flight/commands/AiGenerateInstructionsCommand.php Added phpcs:ignore comment for heredoc syntax
tests/server/index.php Added phpcs:ignore comment for heredoc syntax
tests/commands/RouteCommandTest.php Added phpcs:ignore comments for heredoc syntax
tests/ViewTest.php Added phpcs:ignore comments for heredoc syntax
tests/FlightTest.php Added phpcs:ignore comment for heredoc syntax
Comments suppressed due to low confidence (1)

tests/EngineTest.php:401

  • The exception type has been changed from JsonException to Exception, but this makes the test less specific. The Json utility class wraps JsonException in Exception, so this change may be hiding the actual exception behavior that should be tested.
		$this->expectException(Exception::class);

public static function prettyPrint($data, int $additionalOptions = 0): string
{
$options = self::DEFAULT_ENCODE_OPTIONS | JSON_PRETTY_PRINT | $additionalOptions;
return self::encode($data, $options);
Copy link

Copilot AI Jul 20, 2025

Choose a reason for hiding this comment

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

The prettyPrint method combines options using bitwise OR and then passes them to encode(), which will apply bitwise OR again with DEFAULT_ENCODE_OPTIONS. This could lead to unexpected behavior or option conflicts. Consider calling json_encode directly with the final options.

Suggested change
return self::encode($data, $options);
try {
return json_encode($data, $options, 512);
} catch (JsonException $e) {
throw new Exception('JSON pretty print encoding failed: ' . $e->getMessage(), $e->getCode(), $e);
}

Copilot uses AI. Check for mistakes.
@n0nag0n n0nag0n merged commit b331797 into master Jul 20, 2025
6 checks passed
@n0nag0n n0nag0n deleted the json-util branch July 20, 2025 15:54
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