Add Slice to Csharp --enable-analysis (#4617)

Author: pepone

.github/workflows/ci.yml

@@ -58,11 +58,13 @@ jobs:
             build_flags: "OPTIMIZE=no"
             test_flags: "--swift-config=debug --csharp-config=Debug"
             python_tests: true
+            enable_csharp_analysis: true
           - os: ubuntu-24.04
             config: "debug"
             build_flags: "OPTIMIZE=no"
             test_flags: "--csharp-config=Debug"
             python_tests: true
+            enable_csharp_analysis: true
 
           # TODO - figure out how to properly install debug Python
           - os: windows-2022
@@ -72,6 +74,7 @@ jobs:
             test_flags: "--platform=x64 --config=Debug"
             msbuild_project: "msbuild/ice.proj"
             python_tests: true
+            enable_csharp_analysis: true
 
           # iOS
           - os: macos-26
@@ -171,6 +174,12 @@ jobs:
         shell: powershell
         if: runner.os == 'Windows'
 
+      - name: Enable C# Analysis
+        if: matrix.enable_csharp_analysis == true
+        run: |
+          echo "EnableAnalysis=true" >> $env:GITHUB_ENV
+        shell: pwsh
+
       - name: Build ${{ matrix.config }} on ${{ matrix.os }}
         uses: ./.github/actions/build
         timeout-minutes: 90

cpp/src/slice2cs/CsUtil.cpp

@@ -813,8 +813,8 @@ Slice::Csharp::writeOptionalMarshalUnmarshalCode(
         }
         else
         {
-            out << nl << stream << ".writeSize(" << param << ".Count * "
-                << (keyType->minWireSize() + valueType->minWireSize()) << " + (" << param << ".Count > 254 ? 5 : 1));";
+            out << nl << stream << ".writeSize((" << param << ".Count * "
+                << (keyType->minWireSize() + valueType->minWireSize()) << ") + (" << param << ".Count > 254 ? 5 : 1));";
         }
         writeMarshalUnmarshalCode(out, type, scope, param, marshal, customStream);
         if (keyType->isVariableLength() || valueType->isVariableLength())
@@ -1611,7 +1611,7 @@ Slice::Csharp::writeOptionalSequenceMarshalUnmarshalCode(
             }
             else if (st->minWireSize() > 1)
             {
-                out << nl << stream << ".writeSize(" << length << " * " << st->minWireSize() << " + (" << length
+                out << nl << stream << ".writeSize((" << length << " * " << st->minWireSize() << ") + (" << length
                     << " > 254 ? 5 : 1));";
             }
             writeSequenceMarshalUnmarshalCode(out, seq, scope, param, marshal, true, stream);

cpp/src/slice2cs/Gen.cpp

@@ -758,7 +758,9 @@ Slice::CsVisitor::modulePrefixEnd(const ModulePtr& p)
     }
 }
 
-Slice::Gen::Gen(const string& base, const vector<string>& includePaths, const string& dir) : _includePaths(includePaths)
+Slice::Gen::Gen(const string& base, const vector<string>& includePaths, const string& dir, bool enableAnalysis)
+    : _includePaths(includePaths),
+      _enableAnalysis(enableAnalysis)
 {
     string fileBase = base;
     string::size_type pos = base.find_last_of("/\\");
@@ -781,7 +783,11 @@ Slice::Gen::Gen(const string& base, const vector<string>& includePaths, const st
     }
     FileTracker::instance()->addFile(file);
     printHeader();
-    printGeneratedHeader(_out, fileBase + ".ice");
+
+    if (!_enableAnalysis)
+    {
+        printGeneratedHeader(_out, fileBase + ".ice");
+    }
 
     _out << sp;
     _out << nl << "#nullable enable";
@@ -790,20 +796,21 @@ Slice::Gen::Gen(const string& base, const vector<string>& includePaths, const st
 
     _out << sp;
 
-    /*
-    // Disable some warnings when auto-generated is removed from the header. See printGeneratedHeader above.
-    _out << nl << "#pragma warning disable SA1403 // File may only contain a single namespace";
-    _out << nl << "#pragma warning disable SA1611 // The documentation for parameter x is missing";
+    if (_enableAnalysis)
+    {
+        // Disable some warnings when auto-generated is removed from the header. See printGeneratedHeader above.
+        _out << nl << "#pragma warning disable SA1403 // File may only contain a single namespace";
+        _out << nl << "#pragma warning disable SA1611 // The documentation for parameter x is missing";
 
-    _out << nl << "#pragma warning disable CA1041 // Provide a message for the ObsoleteAttribute that marks ...";
-    _out << nl << "#pragma warning disable CA1068 // Cancellation token as last parameter";
-    _out << nl << "#pragma warning disable CA1725 // Change parameter name istr_ to istr in order to match ...";
+        _out << nl << "#pragma warning disable CA1041 // Provide a message for the ObsoleteAttribute that marks ...";
+        _out << nl << "#pragma warning disable CA1068 // Cancellation token as last parameter";
+        _out << nl << "#pragma warning disable CA1725 // Change parameter name istr_ to istr in order to match ...";
 
-    // Missing doc - only necessary for the tests.
-    _out << nl << "#pragma warning disable SA1602";
-    _out << nl << "#pragma warning disable SA1604";
-    _out << nl << "#pragma warning disable SA1605";
-    */
+        // Missing doc - only necessary for the tests.
+        _out << nl << "#pragma warning disable SA1602";
+        _out << nl << "#pragma warning disable SA1604";
+        _out << nl << "#pragma warning disable SA1605";
+    }
 
     _out << nl << "#pragma warning disable CS1591 // Missing XML Comment";
     _out << nl << "#pragma warning disable CS1573 // Parameter has no matching param tag in the XML comment";
@@ -1112,7 +1119,7 @@ Slice::Gen::TypesVisitor::visitDictionary(const DictionaryPtr& p)
     _out << nl << "public static " << name << " read(Ice.InputStream istr)";
     _out << sb;
     _out << nl << "int sz = istr.readSize();";
-    _out << nl << name << " r = new " << name << "();";
+    _out << nl << "var r = new " << name << "();";
     _out << nl << "for (int i = 0; i < sz; ++i)";
     _out << sb;
     _out << nl << keyS << " k;";
@@ -1581,7 +1588,7 @@ Slice::Gen::TypesVisitor::visitDataMember(const DataMemberPtr& p)
         BuiltinPtr builtin = dynamic_pointer_cast<Builtin>(p->type());
 
         // Don't explicitly initialize to default value.
-        if (!builtin || builtin->kind() == Builtin::KindString || defaultValue != "0")
+        if (!builtin || builtin->kind() == Builtin::KindString || (defaultValue != "0" && defaultValue != "false"))
         {
             _out << " = ";
             writeConstantValue(p->type(), p->defaultValueType(), defaultValue);

cpp/src/slice2cs/Gen.h

@@ -75,7 +75,7 @@ namespace Slice
     class Gen final
     {
     public:
-        Gen(const std::string&, const std::vector<std::string>&, const std::string&);
+        Gen(const std::string&, const std::vector<std::string>&, const std::string&, bool);
         Gen(const Gen&) = delete;
         ~Gen();
 
@@ -84,6 +84,7 @@ namespace Slice
     private:
         IceInternal::Output _out;
         std::vector<std::string> _includePaths;
+        bool _enableAnalysis;
 
         void printHeader();
 

cpp/src/slice2cs/Main.cpp

@@ -105,6 +105,8 @@ usage(const string& n)
                   "--depend                 Generate Makefile dependencies.\n"
                   "--depend-xml             Generate dependencies in XML format.\n"
                   "--depend-file FILE       Write dependencies to FILE instead of standard output.\n"
+                  "--enable-analysis        Enable static analysis for generated code by not including\n"
+                  "                         <auto-generated> tags which suppresses analyzer warnings.\n"
                   "--validate               Validate command line options.\n";
 }
 
@@ -122,6 +124,7 @@ compile(const vector<string>& argv)
     opts.addOpt("", "depend");
     opts.addOpt("", "depend-xml");
     opts.addOpt("", "depend-file", IceInternal::Options::NeedArg, "");
+    opts.addOpt("", "enable-analysis");
     opts.addOpt("d", "debug");
 
     bool validate = find(argv.begin(), argv.end(), "--validate") != argv.end();
@@ -180,6 +183,8 @@ compile(const vector<string>& argv)
 
     string dependFile = opts.optArg("depend-file");
 
+    bool enableAnalysis = opts.isSet("enable-analysis");
+
     bool debug = opts.isSet("debug");
 
     if (sliceFiles.empty())
@@ -251,7 +256,7 @@ compile(const vector<string>& argv)
                 CsharpDocCommentFormatter formatter;
                 parseAllDocComments(unit, formatter);
 
-                Gen gen(preprocessor->getBaseName(), includePaths, output);
+                Gen gen(preprocessor->getBaseName(), includePaths, output, enableAnalysis);
                 gen.generate(unit);
 
                 status |= unit->getStatus();

csharp/src/Directory.Build.props

@@ -17,4 +17,9 @@
         <GlobalAnalyzerConfigFiles Include="$(MSBuildThisFileDirectory)../msbuild/CodeAnalysis.Base.globalconfig" />
         <GlobalAnalyzerConfigFiles Include="$(MSBuildThisFileDirectory)../msbuild/CodeAnalysis.Src.globalconfig" />
     </ItemGroup>
+    <ItemDefinitionGroup>
+        <SliceCompile Condition="'$(EnableAnalysis)' == 'true'">
+            <AdditionalOptions>--enable-analysis</AdditionalOptions>
+        </SliceCompile>
+    </ItemDefinitionGroup>
 </Project>

csharp/test/Directory.Build.props

@@ -18,4 +18,9 @@
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
         </PackageReference>
     </ItemGroup>
+    <ItemDefinitionGroup>
+        <SliceCompile Condition="'$(EnableAnalysis)' == 'true'">
+            <AdditionalOptions>--enable-analysis</AdditionalOptions>
+        </SliceCompile>
+    </ItemDefinitionGroup>
 </Project>

csharp/test/Ice/ami/AllTests.cs

@@ -55,9 +55,9 @@ public bool SentSynchronously
             }
         }
 
-        public void Report(bool sentSynchronously)
+        public void Report(bool value)
         {
-            SentSynchronously = sentSynchronously;
+            SentSynchronously = value;
             Sent = true;
         }
 

csharp/test/Ice/executor/AllTests.cs

@@ -14,7 +14,7 @@ public Progress()
 
         public bool getResult() => _sentSynchronously;
 
-        public void Report(bool sentSynchronously) => _sentSynchronously = sentSynchronously;
+        public void Report(bool value) => _sentSynchronously = value;
 
         private bool _sentSynchronously;
     }

slice/IceGrid/Descriptor.ice

@@ -95,7 +95,7 @@ module IceGrid
         /// The descriptors of well-known objects.
         ObjectDescriptorSeq objects;
 
-        /// The descriptors of allocatable objects
+        /// The descriptors of allocatable objects.
         ObjectDescriptorSeq allocatables;
     }