From 3be48dac81aa73eeb1da04dbeaf1d257adb6bbb3 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 31 Oct 2024 00:12:32 +0100 Subject: [PATCH 1/2] Add MATLAB code analyzer --- .github/actions/setup-dependencies/action.yml | 48 ----------------- .github/actions/setup-matlab/action.yml | 51 +++++++++++++++++++ .github/workflows/ci.yml | 4 ++ .github/workflows/matlab_analyzer.yml | 43 ++++++++++++++++ cpp/src/slice2matlab/Main.cpp | 4 +- matlab/.gitignore | 1 + matlab/config/analyzer.json | 14 +++++ matlab/config/code_analyzer.m | 10 ++++ matlab/lib/+Ice/InputStream.m | 2 +- matlab/lib/+Ice/ObjectPrx.m | 14 ++--- matlab/lib/+Ice/identityToString.m | 2 +- matlab/lib/+IceInternal/EncapsDecoder10.m | 3 +- matlab/lib/+IceInternal/EncapsDecoder11.m | 2 +- matlab/lib/+IceInternal/EncapsEncoder10.m | 2 +- 14 files changed, 137 insertions(+), 63 deletions(-) create mode 100644 .github/actions/setup-matlab/action.yml create mode 100644 .github/workflows/matlab_analyzer.yml create mode 100644 matlab/config/analyzer.json create mode 100644 matlab/config/code_analyzer.m diff --git a/.github/actions/setup-dependencies/action.yml b/.github/actions/setup-dependencies/action.yml index ff283c368b2..fc389dde7f8 100644 --- a/.github/actions/setup-dependencies/action.yml +++ b/.github/actions/setup-dependencies/action.yml @@ -97,54 +97,6 @@ runs: shell: powershell if: runner.os == 'Windows' - # - # MATLAB - # - - name: Setup MATLAB - id: setup-matlab - uses: matlab-actions/setup-matlab@v2 - with: - release: "R2024a" - if: matrix.config == 'matlab' - - - name: Set MATLAB_HOME and MATLAB_VERSION - run: | - # Needed for MATLAB R2024a - sudo apt install -y libgtk2.0-0 - echo "MATLAB_VERSION=R2024a" >> $GITHUB_ENV - echo "MATLAB_HOME=${{ steps.setup-matlab.outputs.matlabroot }}" >> $GITHUB_ENV - shell: bash - if: runner.os == 'Linux' && matrix.config == 'matlab' - - - name: Set MATLAB_HOME and MATLAB_VERSION - run: | - echo "MATLAB_VERSION=R2024a" >> $env:GITHUB_ENV - echo "MATLAB_HOME=${{ steps.setup-matlab.outputs.matlabroot }}" >> $env:GITHUB_ENV - shell: powershell - if: runner.os == 'Windows' && matrix.config == 'matlab' - - # https://github.com/matlab-actions/run-command/issues/53 - - name: Get run-matlab-command - run: | - wget -O /usr/local/bin/run-matlab-command https://ssd.mathworks.com/supportfiles/ci/run-matlab-command/v2/glnxa64/run-matlab-command - chmod +x /usr/local/bin/run-matlab-command - echo "MATLAB_COMMAND=/usr/local/bin/run-matlab-command" >> $GITHUB_ENV - - # https://www.mathworks.com/matlabcentral/answers/1907290-how-to-manually-select-the-libstdc-library-to-use-to-resolve-a-version-glibcxx_-not-found - echo "LD_PRELOAD=/lib/x86_64-linux-gnu/libstdc++.so.6" >> $GITHUB_ENV - - shell: bash - if: runner.os == 'Linux' && matrix.config == 'matlab' - - # Windows is currently not working. We get an error: "'matlab' executable not found on the system path" - # However, MATLAB is installed and the path is (seemingly) set correctly - - name: Get run-matlab-command - run: | - Invoke-WebRequest https://ssd.mathworks.com/supportfiles/ci/run-matlab-command/v2/win64/run-matlab-command.exe -OutFile C:\Windows\System32\run-matlab-command.exe - echo "MATLAB_COMMAND=C:\Windows\System32\run-matlab-command.exe" >> $env:GITHUB_ENV - shell: powershell - if: runner.os == 'Windows' && matrix.config == 'matlab' - # # Cache # diff --git a/.github/actions/setup-matlab/action.yml b/.github/actions/setup-matlab/action.yml new file mode 100644 index 00000000000..fa9c7f1a28c --- /dev/null +++ b/.github/actions/setup-matlab/action.yml @@ -0,0 +1,51 @@ +name: Setup MATLAB +description: Setup MATLAB for use in GitHub Actions +runs: + using: "composite" + steps: + # + # MATLAB + # + - name: Setup MATLAB + id: setup-matlab + uses: matlab-actions/setup-matlab@v2 + with: + release: "R2024a" + + - name: Set MATLAB_HOME and MATLAB_VERSION + run: | + # Needed for MATLAB R2024a + sudo apt install -y libgtk2.0-0 + echo "MATLAB_VERSION=R2024a" >> $GITHUB_ENV + echo "MATLAB_HOME=${{ steps.setup-matlab.outputs.matlabroot }}" >> $GITHUB_ENV + shell: bash + if: runner.os == 'Linux' + + - name: Set MATLAB_HOME and MATLAB_VERSION + run: | + echo "MATLAB_VERSION=R2024a" >> $env:GITHUB_ENV + echo "MATLAB_HOME=${{ steps.setup-matlab.outputs.matlabroot }}" >> $env:GITHUB_ENV + shell: powershell + if: runner.os == 'Windows' + + # https://github.com/matlab-actions/run-command/issues/53 + - name: Get run-matlab-command + run: | + wget -O /usr/local/bin/run-matlab-command https://ssd.mathworks.com/supportfiles/ci/run-matlab-command/v2/glnxa64/run-matlab-command + chmod +x /usr/local/bin/run-matlab-command + echo "MATLAB_COMMAND=/usr/local/bin/run-matlab-command" >> $GITHUB_ENV + + # https://www.mathworks.com/matlabcentral/answers/1907290-how-to-manually-select-the-libstdc-library-to-use-to-resolve-a-version-glibcxx_-not-found + echo "LD_PRELOAD=/lib/x86_64-linux-gnu/libstdc++.so.6" >> $GITHUB_ENV + + shell: bash + if: runner.os == 'Linux' + + # Windows is currently not working. We get an error: "'matlab' executable not found on the system path" + # However, MATLAB is installed and the path is (seemingly) set correctly + - name: Get run-matlab-command + run: | + Invoke-WebRequest https://ssd.mathworks.com/supportfiles/ci/run-matlab-command/v2/win64/run-matlab-command.exe -OutFile C:\Windows\System32\run-matlab-command.exe + echo "MATLAB_COMMAND=C:\Windows\System32\run-matlab-command.exe" >> $env:GITHUB_ENV + shell: powershell + if: runner.os == 'Windows' && matrix.config == 'matlab' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fcda668691..f3d20166dbc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,10 @@ jobs: - name: Setup Dependencies uses: ./.github/actions/setup-dependencies + - name: Setup MATLAB + uses: ./.github/actions/setup-matlab + if: matrix.config == 'matlab' + - name: Build ${{ matrix.config }} on ${{ matrix.os }} uses: ./.github/actions/build timeout-minutes: 90 diff --git a/.github/workflows/matlab_analyzer.yml b/.github/workflows/matlab_analyzer.yml new file mode 100644 index 00000000000..4b31f480f53 --- /dev/null +++ b/.github/workflows/matlab_analyzer.yml @@ -0,0 +1,43 @@ +name: Matlab Analyzer + +on: + workflow_dispatch: + push: + branches: ["main"] + pull_request: + # The branches below must be a subset of the branches above + branches: ["main"] + +jobs: + matlab-analyzer: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup Dependencies + uses: ./.github/actions/setup-dependencies + + - name: Setup Dependencies + uses: ./.github/actions/setup-matlab + + - name: Build MATLAB on Ubuntu + uses: ./.github/actions/build + timeout-minutes: 90 + with: + working_directory: "matlab" + build_cpp_and_python: true + + - name: MATLAB Analyzer + run: | + $MATLAB_COMMAND code_analyzer + working-directory: ./matlab/config + shell: bash + + - name: Upload Analyzer Report + uses: actions/upload-artifact@v4 + with: + name: matlab-analyzer-report + path: ./matlab/config/result.json + if-no-files-found: ignore + if: always() diff --git a/cpp/src/slice2matlab/Main.cpp b/cpp/src/slice2matlab/Main.cpp index 24304695e6d..c73f4d8d4d9 100644 --- a/cpp/src/slice2matlab/Main.cpp +++ b/cpp/src/slice2matlab/Main.cpp @@ -1785,7 +1785,7 @@ CodeVisitor::visitClassDefStart(const ClassDefPtr& p) out.inc(); writeBaseClassArrayParams(out, allMembers, false); out.dec(); - out << nl << "end;"; + out << nl << "end"; out << nl << self << " = " << self << "@" << getAbsolute(base) << "(v{:});"; @@ -1817,7 +1817,7 @@ CodeVisitor::visitClassDefStart(const ClassDefPtr& p) out << nl << self << "." << q->fixedName << " = " << q->fixedName << ';'; } out.dec(); - out << nl << "end;"; + out << nl << "end"; } out.dec(); diff --git a/matlab/.gitignore b/matlab/.gitignore index 5b10f35ff6c..f5042a72765 100644 --- a/matlab/.gitignore +++ b/matlab/.gitignore @@ -5,3 +5,4 @@ lib/x86_64-linux-gnu/ toolbox/build toolbox/toolbox.prj toolbox/info.xml +config/result.json diff --git a/matlab/config/analyzer.json b/matlab/config/analyzer.json new file mode 100644 index 00000000000..7974e5604dc --- /dev/null +++ b/matlab/config/analyzer.json @@ -0,0 +1,14 @@ +{ + "baseConfiguration" : "factory", + "checks": + { + "PROPLC": + { + "enabled" : false + }, + "PROP": + { + "enabled" : false + } + } +} diff --git a/matlab/config/code_analyzer.m b/matlab/config/code_analyzer.m new file mode 100644 index 00000000000..608d21e8d7f --- /dev/null +++ b/matlab/config/code_analyzer.m @@ -0,0 +1,10 @@ +function code_analyzer() + result = codeIssues(["../lib/+Ice", "../lib/+IceInternal", "../lib/generated"], CodeAnalyzerConfiguration="analyzer.json"); + export(result, "result.json", FileFormat="json"); + disp(result); + if height(result.Issues) > 0 + exit(1); + else + exit(0); + end +end diff --git a/matlab/lib/+Ice/InputStream.m b/matlab/lib/+Ice/InputStream.m index 1cbc38a45b9..0022b189ad2 100644 --- a/matlab/lib/+Ice/InputStream.m +++ b/matlab/lib/+Ice/InputStream.m @@ -531,11 +531,11 @@ function skipSlice(obj) end end function r = readOptional(obj, tag, fmt) + import IceInternal.Protocol; %assert(isobject(obj.encapsStack)); if obj.encoding_1_0 r = false; % Optional members aren't supported with the 1.0 encoding. elseif isobject(obj.encapsStackDecoder) - import IceInternal.Protocol; current = obj.encapsStackDecoder.current; if ~isobject(current) || bitand(current.sliceFlags, Protocol.FLAG_HAS_OPTIONAL_MEMBERS) r = readOptionalImpl(tag, fmt, obj.encapsStack.endPos); diff --git a/matlab/lib/+Ice/ObjectPrx.m b/matlab/lib/+Ice/ObjectPrx.m index 981689add96..1d5d84cd1c8 100644 --- a/matlab/lib/+Ice/ObjectPrx.m +++ b/matlab/lib/+Ice/ObjectPrx.m @@ -959,7 +959,7 @@ function iceEndWriteParams(~, os) size = os.buf.size; end - if length(varargin) == 1 + if isscalar(varargin) % % Avoid the string concatenation % @@ -1056,7 +1056,7 @@ function iceEndWriteParams(~, os) size = os.buf.size; end futPtr = libpointer('voidPtr'); % Output param - if length(varargin) == 1 + if isscalar(varargin) % % Avoid the string concatenation % @@ -1116,7 +1116,7 @@ function iceThrowUserException(~, is, varargin) % Varargs are user exception typ function r = uncheckedCast(p, varargin) if isempty(varargin) r = p; - elseif length(varargin) == 1 + elseif isscalar(varargin) if ~isempty(p) r = p.ice_facet(varargin{1}); else @@ -1133,9 +1133,9 @@ function iceThrowUserException(~, is, varargin) % Varargs are user exception typ hasFacet = false; facet = []; context = {}; - if length(varargin) == 1 + if isscalar(varargin) if isa(varargin{1}, 'containers.Map') - context = { varargin{1} }; + context = varargin(1); elseif isempty(varargin{1}) || isa(varargin{1}, 'char') hasFacet = true; facet = varargin{1}; @@ -1145,7 +1145,7 @@ function iceThrowUserException(~, is, varargin) % Varargs are user exception typ elseif length(varargin) == 2 hasFacet = true; facet = varargin{1}; - context = { varargin{2} }; + context = varargin(2); elseif length(varargin) > 2 throw(LocalException('Ice:ArgumentException', 'too many arguments to checkedCast')); end @@ -1167,7 +1167,7 @@ function iceThrowUserException(~, is, varargin) % Varargs are user exception typ function r = iceUncheckedCast(p, cls, varargin) hasFacet = false; facet = []; - if length(varargin) == 1 + if isscalar(varargin) hasFacet = true; facet = varargin{1}; elseif length(varargin) > 1 diff --git a/matlab/lib/+Ice/identityToString.m b/matlab/lib/+Ice/identityToString.m index a5e76a7b478..f87769feb73 100644 --- a/matlab/lib/+Ice/identityToString.m +++ b/matlab/lib/+Ice/identityToString.m @@ -10,7 +10,7 @@ % Copyright (c) ZeroC, Inc. All rights reserved. - if length(varargin) == 1 + if isscalar(varargin) mode = varargin{1}; elseif isempty(varargin) mode = Ice.ToStringMode.Unicode; diff --git a/matlab/lib/+IceInternal/EncapsDecoder10.m b/matlab/lib/+IceInternal/EncapsDecoder10.m index 5c63f9fb5a6..bfa235b3404 100644 --- a/matlab/lib/+IceInternal/EncapsDecoder10.m +++ b/matlab/lib/+IceInternal/EncapsDecoder10.m @@ -119,7 +119,7 @@ function throwException(obj) end end - function startInstance(obj, sliceType) + function startInstance(obj, ~) %assert(obj.sliceType == sliceType); obj.skipFirstSlice = true; end @@ -203,7 +203,6 @@ function readInstance(obj) % obj.startSlice(); mostDerivedId = obj.typeId; - v = []; while true % % For the 1.0 encoding, the type ID for the base Object class diff --git a/matlab/lib/+IceInternal/EncapsDecoder11.m b/matlab/lib/+IceInternal/EncapsDecoder11.m index 6ff00e3f063..5cc6f752ee1 100644 --- a/matlab/lib/+IceInternal/EncapsDecoder11.m +++ b/matlab/lib/+IceInternal/EncapsDecoder11.m @@ -108,7 +108,7 @@ function throwException(obj) end end - function startInstance(obj, sliceType) + function startInstance(obj, ~) %assert(obj.current.sliceType == sliceType); obj.current.skipFirstSlice = true; end diff --git a/matlab/lib/+IceInternal/EncapsEncoder10.m b/matlab/lib/+IceInternal/EncapsEncoder10.m index 8d68ef9cf3c..235d1f4f513 100644 --- a/matlab/lib/+IceInternal/EncapsEncoder10.m +++ b/matlab/lib/+IceInternal/EncapsEncoder10.m @@ -73,7 +73,7 @@ function endSlice(obj) end function writePendingValues(obj) - while length(obj.toBeMarshaledMap) > 0 + while ~isempty(obj.toBeMarshaledMap) % % Consider the to be marshaled instances as marshaled now, % this is necessary to avoid adding again the "to be From 3784bc135e8de0bdef5dc6144884cff0bd386a88 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 31 Oct 2024 14:23:01 +0100 Subject: [PATCH 2/2] Call setup-matlab from setup-dependencies --- .github/actions/setup-dependencies/action.yml | 15 +++++++++++++++ .github/workflows/ci.yml | 6 ++---- .github/workflows/matlab_analyzer.yml | 5 ++--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/.github/actions/setup-dependencies/action.yml b/.github/actions/setup-dependencies/action.yml index fc389dde7f8..9e41409a609 100644 --- a/.github/actions/setup-dependencies/action.yml +++ b/.github/actions/setup-dependencies/action.yml @@ -1,6 +1,14 @@ name: Setup Dependencies inputs: + use_matlab: + description: "Indicates whether to install and configure MATLAB" + type: choice + required: true + default: "false" + options: + - true + - false use_ccache: description: "Indicates whether to install and configure ccache" type: choice @@ -97,6 +105,13 @@ runs: shell: powershell if: runner.os == 'Windows' + # + # MATLAB + # + - name: Setup Dependencies + uses: ./.github/actions/setup-matlab + if: inputs.use_matlab == 'true' + # # Cache # diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3d20166dbc..9eb79c8d506 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,10 +89,8 @@ jobs: - name: Setup Dependencies uses: ./.github/actions/setup-dependencies - - - name: Setup MATLAB - uses: ./.github/actions/setup-matlab - if: matrix.config == 'matlab' + with: + use_matlab: ${{ matrix.config == 'matlab' }} - name: Build ${{ matrix.config }} on ${{ matrix.os }} uses: ./.github/actions/build diff --git a/.github/workflows/matlab_analyzer.yml b/.github/workflows/matlab_analyzer.yml index 4b31f480f53..120a3d25107 100644 --- a/.github/workflows/matlab_analyzer.yml +++ b/.github/workflows/matlab_analyzer.yml @@ -17,9 +17,8 @@ jobs: - name: Setup Dependencies uses: ./.github/actions/setup-dependencies - - - name: Setup Dependencies - uses: ./.github/actions/setup-matlab + with: + use_matlab: true - name: Build MATLAB on Ubuntu uses: ./.github/actions/build