search

Esri / coordinate-conversion-addin-dotnet

Addin for ArcMap and Pro for convenient coordinate conversion in Desktop.
Apache License 2.0
45 stars 20 forks source link

Static Code Analyzer : Null Dereference #436

Closed jmccausland closed 6 years ago

jmccausland commented 6 years ago

Tasks to be done July 16 2018

Update homepage - whats new

Create a Aug 2018 CMS topic

Documentation Review for CC

Original Information

the SCA report shows there are possible Null Dereference issues in the code.

Most of the flagged code is due to query interfaces via the 'as' keyword. When it is safe to do a direct cast to an Esri API Interface, we should do that. Everywhere else, null check and refactor if needed.

The ReSharper tool can assist with this exercise and minimize these issues in the future.

lfunkhouser commented 6 years ago

@csmoore please update with the progress made on this issue.

csmoore commented 6 years ago

I completed the high-priority issues from the Fortify Report in branch https://github.com/Esri/coordinate-conversion-addin-dotnet/tree/csm/%23436-SCA-Issues

I am just sanity testing and looking at the other SCA-related issues: ( #437 - #445 - Note: these issues were classified as "Low" in the report)

csmoore commented 6 years ago

Addressed in PR #509

ACueva commented 6 years ago

@dfoll and I discovered there are still warnings being thrown in the Jenkins build. This is occurring both in the single Coordinate Conversion add-in as well as the MT build. We are investigating. FYI @kgonzago

csmoore commented 6 years ago

@ACueva @dfoll we saw something similar in https://github.com/Esri/distance-direction-addin-dotnet/issues/509 and I did notice a couple of non-compile related warning (the build is set to fail if there is a compiler warning): 

coordinate-conversion-addin-dotnet\source\CoordinateConversion\ArcMapAddinCoordinateConversion\Config.esriaddinx : warning : The 'language' attribute in the AddIn element is invalid - The value 'CLR4.5'  is invalid according to the project's target framework version - The value should be 'CLR'.
Microsoft.Common.CurrentVersion.targets(1820,5): warning MSB3270: There was a mismatch between the processor architecture of the project being built "MSIL" and the processor architecture of the reference "ESRI.ArcGIS.ADF.Connection.Local, Version=10.3.0.0, Culture=neutral, PublicKeyToken=8fc3cc631e44ad86, processorArchitecture=x86", "x86". This mismatch may cause runtime failures. Please consider changing the targeted processor architecture of your project through the Configuration Manager so as to align the processor architectures between your project and references, or take a dependency on references with a processor architecture that matches the targeted processor architecture of your project.

So hopefully this is the issue you were seeing - this is addressed in #510

dfoll commented 6 years ago
dfoll commented 6 years ago

@lfunkhouser will we be doing the work in this release to move to a single 2.1/2.2 add-in for CC and MT?

If the goal is to have a single build for 2.1/2.2 for this release, this testing is impeded until we are building this way

csmoore commented 6 years ago

@ACueva @dfoll - those additional warnings you showed me in military tools toolbar build should hopefully be addressed in PR: https://github.com/Esri/military-tools-desktop-addins/pull/61

ACueva commented 6 years ago

@csmoore Found the following warning in the latest MT build:

Build succeeded.

"c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.sln" (default target) (1) ->
"c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.csproj" (default target) (2) ->
(ResolveAssemblyReferences target) -> 
  C:\Program Files (x86)\MSBuild\14.0\bin\Microsoft.Common.CurrentVersion.targets(1820,5): warning MSB3270: There was a mismatch between the processor architecture of the project being built "MSIL" and the processor architecture of the reference "c:\jProd\workspace\MT_Pipeline\coordinate-conversion-addin-dotnet\source\CoordinateConversion\ProAppCoordConversionModule\bin\Release\CoordinateConversion.dll", "AMD64". This mismatch may cause runtime failures. Please consider changing the targeted processor architecture of your project through the Configuration Manager so as to align the processor architectures between your project and references, or take a dependency on references with a processor architecture that matches the targeted processor architecture of your project. [c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.csproj]

"c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.sln" (default target) (1) ->
"c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.csproj" (default target) (2) ->
(PackageArcGISContents target) -> 
  C:\Program Files\ArcGIS\Pro\bin\Esri.ProApp.SDK.Desktop.targets(77,5): warning :  was not found. The value of the defaultAssembly attribute in the Config.daml should match the name of the of the assembly for the add-in project. [c:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcGISPro\MilitaryToolsForArcGISPro.csproj]

    2 Warning(s)
    0 Error(s)

ArcMap comes back with:

Done Building Project "C:\jProd\workspace\MT_Pipeline\military-tools-desktop-addins\source\MilitaryToolsForArcMap\MilitaryToolsForArcMap.csproj" (default targets).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:29.27
-----------------------------------------------------------------------------
Finished Building Military Tools for ArcMap
csmoore commented 6 years ago

@ACueva - those warnings are indeed strange and I don't think I have seen - but are coming from the addin generator and not the compiler so can be safely ignored for the purposes of this issue.

Some of these addin warnings we may have to put up with until we can build on a version of the ArcObject SDK newer than 10.3.1

topowright-zz commented 6 years ago

Currently my understanding on what we need to do to finish this item is to test the addin and the component. The last bit of testing that needs to be done is ArcGIS Pro 2.2

lfunkhouser commented 6 years ago

Yes @dfoll. This release will have a single add-in for Coordinate Conversion that supports both Pro 2.1 and Pro 2.2.