Skip to content

WIP #2802

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

Draft
wants to merge 83 commits into
base: master
Choose a base branch
from
Draft

WIP #2802

wants to merge 83 commits into from

Conversation

sdottaka
Copy link
Member

@sdottaka sdottaka commented Jun 3, 2025

No description provided.

@@ -16,9 +16,14 @@
// DateTime
#include "Clock.cpp"
#include "DateTime.cpp"
#include "DateTimeFormat.cpp"
#include "DateTimeFormatter.cpp"

Check notice

Code scanning / CodeQL

Include header files only Note

The #include pre-processor directive should only be used to include header files.

Copilot Autofix

AI 16 days ago

To fix the issue, we need to refactor the code to include only header files in Foundation.cpp. This involves the following steps:

  1. Move the declarations of classes, functions, and other interfaces from the .cpp files (e.g., DateTimeFormatter.cpp) to corresponding .h header files (e.g., DateTimeFormatter.h).
  2. Ensure that the .cpp files contain only the implementation of the declarations in the header files.
  3. Replace the #include directives for .cpp files in Foundation.cpp with #include directives for the corresponding .h files.

This approach ensures that Foundation.cpp adheres to the principle of including only header files and avoids exposing implementation details unnecessarily.


Suggested changeset 1
Externals/poco/Foundation/src/Foundation.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Externals/poco/Foundation/src/Foundation.cpp b/Externals/poco/Foundation/src/Foundation.cpp
--- a/Externals/poco/Foundation/src/Foundation.cpp
+++ b/Externals/poco/Foundation/src/Foundation.cpp
@@ -19,3 +19,3 @@
 #include "DateTimeFormat.cpp"
-#include "DateTimeFormatter.cpp"
+#include "DateTimeFormatter.h"
 #include "DateTimeParser.cpp"
EOF
@@ -19,3 +19,3 @@
#include "DateTimeFormat.cpp"
#include "DateTimeFormatter.cpp"
#include "DateTimeFormatter.h"
#include "DateTimeParser.cpp"
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +131 to +134
switch (yych) {
case '=': goto yy36;
default: goto yy2;
}

Check notice

Code scanning / CodeQL

No trivial switch statements Note

This switch statement should either handle more cases, or be rewritten as an if statement.

Copilot Autofix

AI about 20 hours ago

To fix the issue, the trivial switch statement on line 131 should be replaced with an if/else statement. Specifically:

  • Replace the switch statement with an if condition to check for the single non-default case (yych == '=').
  • Use an else block to handle the default case.

This change will simplify the control flow while preserving the existing functionality.


Suggested changeset 1
Src/FilterEngine/FilterLexer.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Src/FilterEngine/FilterLexer.cpp b/Src/FilterEngine/FilterLexer.cpp
--- a/Src/FilterEngine/FilterLexer.cpp
+++ b/Src/FilterEngine/FilterLexer.cpp
@@ -130,5 +130,6 @@
 	yych = *++YYCURSOR;
-	switch (yych) {
-		case '=': goto yy36;
-		default: goto yy2;
+	if (yych == '=') {
+		goto yy36;
+	} else {
+		goto yy2;
 	}
EOF
@@ -130,5 +130,6 @@
yych = *++YYCURSOR;
switch (yych) {
case '=': goto yy36;
default: goto yy2;
if (yych == '=') {
goto yy36;
} else {
goto yy2;
}
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +412 to +419
INSTANTIATE_TEST_SUITE_P(
OptimizationCases,
FilterExpressionTest,
::testing::Values(
FilterTestParam{ true },
FilterTestParam{ false }
)
);

Check notice

Code scanning / CodeQL

Unused static function Note

Static function gtest_OptimizationCasesFilterExpressionTest_EvalGenerateName_ is unreachable (
gtest_OptimizationCasesFilterExpressionTest_dummy_
must be removed at the same time)
Static function gtest_OptimizationCasesFilterExpressionTest_EvalGenerator_ is unreachable (
gtest_OptimizationCasesFilterExpressionTest_dummy_
must be removed at the same time)

Copilot Autofix

AI 5 days ago

To fix the issue, the unused static function generated by the INSTANTIATE_TEST_SUITE_P macro should be removed. This involves removing the macro invocation itself, as it is responsible for generating the unused function. Additionally, any associated test cases or parameters that rely on this macro should be reviewed and removed if they are redundant or unnecessary.

Steps to fix:

  1. Remove the INSTANTIATE_TEST_SUITE_P macro invocation on line 670 and its associated parameters.
  2. Ensure that the removal does not affect other parts of the test suite or the functionality of the codebase.
  3. Verify that the remaining tests still execute correctly and cover the intended functionality.
Suggested changeset 1
Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp b/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
--- a/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
+++ b/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
@@ -669,10 +669,3 @@
 
-INSTANTIATE_TEST_SUITE_P(
-	OptimizationCases,
-	FilterExpressionTest,
-	::testing::Values(
-		FilterTestParam{ true },
-		FilterTestParam{ false }
-	)
-);
+// Removed unused INSTANTIATE_TEST_SUITE_P macro invocation.
 
EOF
@@ -669,10 +669,3 @@

INSTANTIATE_TEST_SUITE_P(
OptimizationCases,
FilterExpressionTest,
::testing::Values(
FilterTestParam{ true },
FilterTestParam{ false }
)
);
// Removed unused INSTANTIATE_TEST_SUITE_P macro invocation.

Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +412 to +419
INSTANTIATE_TEST_SUITE_P(
OptimizationCases,
FilterExpressionTest,
::testing::Values(
FilterTestParam{ true },
FilterTestParam{ false }
)
);

Check notice

Code scanning / CodeQL

Unused static variable Note

Static variable gtest_OptimizationCasesFilterExpressionTest_dummy_ is never read.

Copilot Autofix

AI about 20 hours ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.


}

TEST_P(FilterExpressionTest, FileAttributes)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 127 lines.

Copilot Autofix

AI 5 days ago

To fix the issue, we will add comments to document the purpose and logic of the FilterExpressionTest, FileAttributes function. This includes explaining the setup of contexts, the initialization of test data, and the purpose of each major block of code. The comments will provide clarity on what the function is testing and how it achieves its goals, without altering the existing functionality.

Suggested changeset 1
Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp b/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
--- a/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
+++ b/Testing/GoogleTest/FilterEngine/FilterExpression_test.cpp
@@ -384,6 +384,13 @@
 {
+	// Set up the path context with multiple source directories.
 	PathContext paths(L"C:\\dev\\winmerge\\src", L"D:\\dev\\winmerge\\src", L"E:\\dev\\winmerge\\src");
+	
+	// Initialize the diff context for the test.
 	CDiffContext ctxt(paths, 0);
+	
+	// Create and configure a DIFFITEM object to represent file attributes and metadata.
 	DIFFITEM di;
-	int tzd;
+	int tzd; // Timezone offset variable.
+	
+	// Configure the first file's attributes and metadata.
 	di.diffFileInfo[0].path = L"abc";
@@ -392,2 +399,4 @@
 	di.diffFileInfo[0].flags.attributes = FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_ARCHIVE;
+	
+	// Parse and set the file's modification and creation timestamps.
 	Poco::DateTime dt0 = Poco::DateTimeParser::parse("%Y-%m-%d %H:%M:%S", "2025-05-16 15:34:56", tzd);
@@ -396,4 +405,8 @@
 	di.diffFileInfo[0].ctime = dt0.timestamp();
+	
+	// Set the file's encoding and version information.
 	di.diffFileInfo[0].encoding.SetCodepage(65001);
 	di.diffFileInfo[0].version.SetFileVersion(0x00020010, 0x00300002);
+	
+	// Configure additional files for the test.
 	di.diffFileInfo[1].path = L"abc";
EOF
@@ -384,6 +384,13 @@
{
// Set up the path context with multiple source directories.
PathContext paths(L"C:\\dev\\winmerge\\src", L"D:\\dev\\winmerge\\src", L"E:\\dev\\winmerge\\src");

// Initialize the diff context for the test.
CDiffContext ctxt(paths, 0);

// Create and configure a DIFFITEM object to represent file attributes and metadata.
DIFFITEM di;
int tzd;
int tzd; // Timezone offset variable.

// Configure the first file's attributes and metadata.
di.diffFileInfo[0].path = L"abc";
@@ -392,2 +399,4 @@
di.diffFileInfo[0].flags.attributes = FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_ARCHIVE;

// Parse and set the file's modification and creation timestamps.
Poco::DateTime dt0 = Poco::DateTimeParser::parse("%Y-%m-%d %H:%M:%S", "2025-05-16 15:34:56", tzd);
@@ -396,4 +405,8 @@
di.diffFileInfo[0].ctime = dt0.timestamp();

// Set the file's encoding and version information.
di.diffFileInfo[0].encoding.SetCodepage(65001);
di.diffFileInfo[0].version.SetFileVersion(0x00020010, 0x00300002);

// Configure additional files for the test.
di.diffFileInfo[1].path = L"abc";
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +228 to +231
switch (yych) {
case '=': goto yy46;
default: goto yy18;
}

Check notice

Code scanning / CodeQL

No trivial switch statements Note

This switch statement should either handle more cases, or be rewritten as an if statement.

Copilot Autofix

AI about 20 hours ago

To fix the issue, the trivial switch statement on line 228 should be replaced with an if/else statement. This simplifies the control flow while maintaining the same functionality. The replacement will involve checking the value of yych using an if condition for = and an else block for the default case.


Suggested changeset 1
Src/FilterEngine/FilterLexer.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Src/FilterEngine/FilterLexer.cpp b/Src/FilterEngine/FilterLexer.cpp
--- a/Src/FilterEngine/FilterLexer.cpp
+++ b/Src/FilterEngine/FilterLexer.cpp
@@ -227,5 +227,6 @@
 	yych = *++YYCURSOR;
-	switch (yych) {
-		case '=': goto yy46;
-		default: goto yy18;
+	if (yych == '=') {
+		goto yy46;
+	} else {
+		goto yy18;
 	}
EOF
@@ -227,5 +227,6 @@
yych = *++YYCURSOR;
switch (yych) {
case '=': goto yy46;
default: goto yy18;
if (yych == '=') {
goto yy46;
} else {
goto yy18;
}
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +238 to +241
switch (yych) {
case '=': goto yy47;
default: goto yy20;
}

Check notice

Code scanning / CodeQL

No trivial switch statements Note

This switch statement should either handle more cases, or be rewritten as an if statement.

Copilot Autofix

AI about 20 hours ago

To fix the issue, the trivial switch statement on line 238 should be replaced with an equivalent if/else structure. Specifically:

  • The case for '=' should be converted into an if condition.
  • The default case should be converted into an else block.

This change will simplify the control flow while preserving the existing functionality. The replacement should be made directly in the Src/FilterEngine/FilterLexer.cpp file.


Suggested changeset 1
Src/FilterEngine/FilterLexer.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Src/FilterEngine/FilterLexer.cpp b/Src/FilterEngine/FilterLexer.cpp
--- a/Src/FilterEngine/FilterLexer.cpp
+++ b/Src/FilterEngine/FilterLexer.cpp
@@ -237,5 +237,6 @@
 	yych = *++YYCURSOR;
-	switch (yych) {
-		case '=': goto yy47;
-		default: goto yy20;
+	if (yych == '=') {
+		goto yy47;
+	} else {
+		goto yy20;
 	}
EOF
@@ -237,5 +237,6 @@
yych = *++YYCURSOR;
switch (yych) {
case '=': goto yy47;
default: goto yy20;
if (yych == '=') {
goto yy47;
} else {
goto yy20;
}
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +248 to +251
switch (yych) {
case '=': goto yy48;
default: goto yy22;
}

Check notice

Code scanning / CodeQL

No trivial switch statements Note

This switch statement should either handle more cases, or be rewritten as an if statement.

Copilot Autofix

AI about 20 hours ago

To fix the issue, the trivial switch statement on line 248 should be replaced with an equivalent if/else structure. Specifically:

  • Replace the switch statement with an if condition to check for the single non-default case (yych == '=').
  • Use an else block to handle the default case (goto yy22).

This change will simplify the control flow while preserving the original behavior.


Suggested changeset 1
Src/FilterEngine/FilterLexer.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Src/FilterEngine/FilterLexer.cpp b/Src/FilterEngine/FilterLexer.cpp
--- a/Src/FilterEngine/FilterLexer.cpp
+++ b/Src/FilterEngine/FilterLexer.cpp
@@ -247,5 +247,6 @@
 	yych = *++YYCURSOR;
-	switch (yych) {
-		case '=': goto yy48;
-		default: goto yy22;
+	if (yych == '=') {
+		goto yy48;
+	} else {
+		goto yy22;
 	}
EOF
@@ -247,5 +247,6 @@
yych = *++YYCURSOR;
switch (yych) {
case '=': goto yy48;
default: goto yy22;
if (yych == '=') {
goto yy48;
} else {
goto yy22;
}
Copilot is powered by AI and may make mistakes. Always verify output.
@@ -140,11 +153,8 @@

}

TEST_F(FileFilterHelperTest, SetMask)
TEST_F(FileFilterHelperTest, SetMask2)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 113 lines.

Copilot Autofix

AI about 20 hours ago

To fix the issue, we will add comments to document the purpose of the SetMask2 test function and its individual test cases. The comments will explain the expected behavior of the SetMask method for different inputs and the reasoning behind the assertions. This will make the function easier to understand and maintain without altering its functionality.


Suggested changeset 1
Testing/GoogleTest/FileFilter/FileFilterHelper_test.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Testing/GoogleTest/FileFilter/FileFilterHelper_test.cpp b/Testing/GoogleTest/FileFilter/FileFilterHelper_test.cpp
--- a/Testing/GoogleTest/FileFilter/FileFilterHelper_test.cpp
+++ b/Testing/GoogleTest/FileFilter/FileFilterHelper_test.cpp
@@ -157,4 +157,6 @@
 	{
+		// Test case 1: SetMask with an empty string.
+		// Expect all files and directories to be included, as no specific mask is applied.
 		m_fileFilterHelper.SetMask(_T(""));
-		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));
+		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));  // Any file should be included.
 		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.cpp")));
@@ -162,3 +164,3 @@
 		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("")));
-		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));
+		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));      // Any directory should be included.
 		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("svn")));
@@ -167,7 +169,9 @@
 
+		// Test case 2: SetMask with "*.c".
+		// Expect only files with the ".c" extension to be included.
 		m_fileFilterHelper.SetMask(_T("*.c"));
-		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));
-		EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.cpp")));
+		EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));  // Files with ".c" extension should be included.
+		EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.cpp"))); // Files with other extensions should not be included.
 		EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.ext")));
-		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));
+		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));      // Directories are not affected by file masks.
 		EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("a")));
EOF
@@ -157,4 +157,6 @@
{
// Test case 1: SetMask with an empty string.
// Expect all files and directories to be included, as no specific mask is applied.
m_fileFilterHelper.SetMask(_T(""));
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c"))); // Any file should be included.
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.cpp")));
@@ -162,3 +164,3 @@
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("")));
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T(""))); // Any directory should be included.
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("svn")));
@@ -167,7 +169,9 @@

// Test case 2: SetMask with "*.c".
// Expect only files with the ".c" extension to be included.
m_fileFilterHelper.SetMask(_T("*.c"));
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c")));
EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.cpp")));
EXPECT_EQ(true, m_fileFilterHelper.includeFile(_T("a.c"))); // Files with ".c" extension should be included.
EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.cpp"))); // Files with other extensions should not be included.
EXPECT_EQ(false, m_fileFilterHelper.includeFile(_T("a.ext")));
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("")));
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T(""))); // Directories are not affected by file masks.
EXPECT_EQ(true, m_fileFilterHelper.includeDir(_T("a")));
Copilot is powered by AI and may make mistakes. Always verify output.
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.

1 participant