Platform Specific Analyzer #2083

hermitdave · 2018-05-11T14:37:56Z

Issue: #1497

PR Type

What kind of change does this PR introduce?

<- Feature >

What is the current behavior?

No platform / version analyzers

What is the new behavior?

Added Platform Specific Differences Generator and Roslyn code Analyzer + Fixer

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Docs have been added/updated which fit documentation template. (for bug fixes / features)
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

…alyzer and documentation

Types not present are existing types added in previous SDK versions

nmetulev

Tried it on this code and the warning showed up as expected:

LearningModelPreview learningModel = await LearningModelPreview.LoadModelFromStreamAsync(null);

However, the fix was incomplete:

Related, this line does not generate a warning:

LearningModelPreview learningModel;

should it?

nmetulev · 2018-05-14T01:22:57Z

docs/platform-specific/PlatformSpecificAnalyzer.md

+
+# Platform Specific Analyzer
+
+The [writing version adaptive](https://docs.microsoft.com/windows/uwp/debug-test-perf/version-adaptive-code) code, the developers should ensure that code checks for presence of API before calling it.


Weird wording, should be

When writing version adaptive code...

nmetulev · 2018-05-14T01:23:28Z

docs/platform-specific/PlatformSpecificAnalyzer.md

+| Device family | Universal, 10.0.15063.0 or higher   |
+| ---------------------------------------------------------------- | ----------------------------------- |
+| Namespace                                                        | Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer |
+| NuGet package | [Microsoft.Toolkit.Uwp.UI.Animations](https://www.nuget.org/packages/Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer/) |


Nuget package name is wrong

fixing that too

nmetulev · 2018-05-14T01:41:00Z

docs/platform-specific/PlatformSpecificAnalyzer.md

+
+## Installation
+
+The analyzer is available both as a nuget package and also as Visual Studio Extention


Could not find the Visual Studio Extension, how do we build it?

There is a vsix solution and building it is as easy as including it in sln however that throws VSTS errors. Need to get that looked into.

Got it. Not familiar with VSIX - what's the benefit over having a nuget package? Should we publish both?

I'm sure we can get it building in VSTS, but in general, I don't think it's as good since it requires everyone to install it. Nice part of nuget analyzers is that they are brought in with the solution so clone-open just works.

The VSIX will also need to be added to the list of items sent to the signing service. (and ensure it has the proper filter configuration)

isn't that the reason the analyzer template supports both vsix and nuget ?

Sure, but that's a template, doesn't mean you have to support both methods. Most analyzers that I know of just use NuGet.

I am okay with whatever you and others think 👍 I have no idea about the sort of usage they have plus VSIX would need submissions to VS gallery etc.. maybe its not worth the effort unless there's a strong demand.

We can go nuget to start with. I will fix the docs

nmetulev · 2018-05-14T01:47:12Z

Microsoft.Toolkit.Uwp.PlatformDifferencesGen/Program.cs

+{
+    public class Program
+    {
+        private static string path = @"D:\UwpApi";


Path is hardcoded, should be current directory instead

nmetulev · 2018-05-14T01:48:21Z

...t.Toolkit.Uwp.PlatformSpecificAnalyzer/Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer.csproj

+  <PropertyGroup>
+    <PackageId>Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer</PackageId>
+    <PackageVersion>1.0.0.0</PackageVersion>
+    <Authors>hermi</Authors>


Change author, image, and other properties to match other projects

Add ability to analyse field declaration Correct guard instantiation Correct documentation Correct path in diff generator Correct project file properties

nmetulev · 2018-05-16T03:08:42Z

...t.Toolkit.Uwp.PlatformSpecificAnalyzer/Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer.csproj

@@ -6,22 +6,6 @@
    <GeneratePackageOnBuild>True</GeneratePackageOnBuild>
  </PropertyGroup>

-  <PropertyGroup>


You should still have <Title> <Description> and <PackageTags> here (see other projects for example)

hermitdave · 2018-05-16T05:03:59Z

Beauty of VSIX is install once and use across projects. Nuget is per project. In this instance is prefer VSIX over Nuget

Maintain differences dictionary to prevent frequent loading from resource.

hermitdave · 2018-05-16T10:07:16Z

This is feature complete I believe.
I have tested it with Types, Methods, Properties, Events for Version specific
And tested it with Type for Platform specific.

…o HD-PSA

hermitdave · 2018-05-18T18:47:31Z

@azchohfi any good?

azchohfi · 2018-05-18T21:19:14Z

...t.Toolkit.Uwp.PlatformSpecificAnalyzer/Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer.csproj

+
+  <PropertyGroup>
+    <TargetFramework>netstandard1.3</TargetFramework><Title>Windows Community Toolkit UI</Title>
+    <Description>This .NET standard library provides analyzer and code fixer to ensure that version / platform specific code is well guarded. It is part of the Windows Community Toolkit.</Description>


"code fixes"

Nope I think the current wording is okay. There is an analyser and code fixer in the project

azchohfi · 2018-05-18T21:22:15Z

...t.Toolkit.Uwp.PlatformSpecificAnalyzer/Microsoft.Toolkit.Uwp.PlatformSpecificAnalyzer.csproj

+    <Title>Windows Community Toolkit UWP Platform Specific Analyzer</Title>
+
+    <!-- This is a temporary workaround for https://github.com/dotnet/sdk/issues/955 -->
+    <DebugType>Full</DebugType>


Haven't this been fixed in the latest VS? Oren even opened an issue to remove the workaround on other Toolkit projects #1951

I'm not sure. @nmetulev asked me to make it same as other projects so I just copied that bit as was

@onovotny, have you verified this is fixed now - should we remove it?

This could be merged today and a separate PR cleaning up all projects would be a good idea if applicable

I don't see a problem. Let's merge it.

hermitdave added 4 commits May 11, 2018 15:34

Adding support for Platform Specific Differences generator, Roslyn an…

23b1208

…alyzer and documentation

Code refactoring, class / method XML comments

9ca1b9e

remove vsix solution from build

4f5694d

Tweak to ensure that Analyzer only warns for new types.

11f9d8c

Types not present are existing types added in previous SDK versions

hermitdave requested review from nmetulev, dotMorten, clairernovotny and skendrot May 11, 2018 16:56

hermitdave added 3 commits May 12, 2018 21:02

Support for VisualBasic - Analyzer and CodeFixer added

41c5b2a

Update documentation for VB support

dae808b

Use internal enum instead of nullable bool to indicate type presence

71dc014

nmetulev requested changes May 14, 2018

View reviewed changes

nmetulev added helpers ✋ reviewers wanted labels May 14, 2018

nmetulev added this to the 3.0 milestone May 14, 2018

Address issues raised by @nmetulev

b861255

Add ability to analyse field declaration Correct guard instantiation Correct documentation Correct path in diff generator Correct project file properties

hermitdave force-pushed the HD-PSA branch from c42a90f to b861255 Compare May 14, 2018 15:58

Added support for Enum types in generated diffs.

62e8e92

nmetulev reviewed May 16, 2018

View reviewed changes

hermitdave added 2 commits May 16, 2018 10:30

Support for Events

8a87706

Minor optimisation

2d29b90

Maintain differences dictionary to prevent frequent loading from resource.

hermitdave and others added 3 commits May 16, 2018 13:55

Fix the project properties

9a1175f

Removed VSIX project and updated the analyser doc

50403ed

Merge branch 'master' into HD-PSA

1369f8c

nmetulev approved these changes May 17, 2018

View reviewed changes

hermitdave added 2 commits May 17, 2018 17:54

clean up and use right param rather than substring

e5e5f2b

Merge branch 'HD-PSA' of github.com:Microsoft/UWPCommunityToolkit int…

0d42986

…o HD-PSA

azchohfi self-requested a review May 18, 2018 17:40

Merge branch 'master' into HD-PSA

68d3e7c

azchohfi reviewed May 18, 2018

View reviewed changes

Merge branch 'master' into HD-PSA

e1be752

azchohfi merged commit 0d970c6 into master May 21, 2018

azchohfi deleted the HD-PSA branch May 21, 2018 19:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Platform Specific Analyzer #2083

Platform Specific Analyzer #2083

hermitdave commented May 11, 2018 •

edited by nmetulev

nmetulev left a comment •

edited

nmetulev May 14, 2018

hermitdave May 14, 2018

nmetulev May 14, 2018

hermitdave May 14, 2018

nmetulev May 14, 2018

hermitdave May 14, 2018

nmetulev May 16, 2018

clairernovotny May 16, 2018

clairernovotny May 16, 2018 •

edited

hermitdave May 16, 2018

clairernovotny May 16, 2018

hermitdave May 16, 2018

nmetulev May 14, 2018

nmetulev May 14, 2018

nmetulev May 16, 2018 •

edited

hermitdave commented May 16, 2018

hermitdave commented May 16, 2018

hermitdave commented May 18, 2018

azchohfi May 18, 2018

hermitdave May 19, 2018

azchohfi May 18, 2018

hermitdave May 19, 2018

nmetulev May 20, 2018

hermitdave May 21, 2018

azchohfi May 21, 2018


		# Platform Specific Analyzer

		The [writing version adaptive](https://docs.microsoft.com/windows/uwp/debug-test-perf/version-adaptive-code) code, the developers should ensure that code checks for presence of API before calling it.


		## Installation

		The analyzer is available both as a nuget package and also as Visual Studio Extention

Platform Specific Analyzer #2083

Platform Specific Analyzer #2083

Conversation

hermitdave commented May 11, 2018 • edited by nmetulev

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

nmetulev left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clairernovotny May 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmetulev May 16, 2018 • edited

Choose a reason for hiding this comment

hermitdave commented May 16, 2018

hermitdave commented May 16, 2018

hermitdave commented May 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hermitdave commented May 11, 2018 •

edited by nmetulev

nmetulev left a comment •

edited

clairernovotny May 16, 2018 •

edited

nmetulev May 16, 2018 •

edited