Enforce code style guidelines in .editorconfig

[mvegaca]

I've seen that we have some class members with methods in inline get/set methods and others with classic get/ set methods.

i.e. MasterDetailPage Selected property in ViewModel

Mvvm Basic Framework

private SampleOrder _selected;

public SampleOrder Selected
{
   get { return _selected; }
   set { Set(ref _selected, value); }
}

Prism Framework

private SampleOrder _selected;

public SampleOrder Selected
{
   get => _selected;
   set => SetProperty(ref _selected, value);
}

I think we should use C# 7.0 inline methods.

[mrlacey]

If I remember correctly, this was discussed before and we opted to not use the inline syntax because there would be places where a getter or setter would do more than just set or return the property and so inline would be inappropriate and we wanted consistency throughout.

A quick look through the templates and both syntax are used throughout projects with all frameworks. It looks like the new syntax is used predominantly in the templates created more recently but this isn't true in all instances.

This from WebViewPageViewModel.cs in the Prism templates.

        public bool IsLoading
        {
            get
            {
                 return _isLoading;
            }

            set
            {
                if (value)
                {
                    IsShowingFailedMessage = false;
                }

                Param_Setter(ref _isLoading, value);
                IsLoadingVisibility = value ? Visibility.Visible : Visibility.Collapsed;
            }
        }

If the inline syntax isn't appropriate (as in the example above), should the getter still use the inline syntax? Or should it match what's used for the setter?

If we're going to set a standard that should be followed, automated tests must be created to ensure this is followed in the future and the generated code is consistent going forward.

If we're going to ensure consistency of this then also need to review consistency in other parts of the code.

Also, need to ensure consistency in the VB templates too.

[crutkas]

Did we get a resolution on this?

[mrlacey]

Without any strong arguments for making a change, I suggest closing this and keeping the code standards we have at the moment.

[sibille]

I think we should adapt the Prism templates to meet the same standard as we do for others. Regarding the tests that will ensure code standards in the future, this can be done with .editorconfig (https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference), but the rules that enforce this seems to be broken (https://github.com/dotnet/roslyn/issues/26276)

[mikebattista]

Assigning to consider how we can leverage .editorconfig to define and enforce code style guidelines in the WinUI templates going forward.

[mrlacey]

My inclination is to follow the default guidelines recommended by the tools, languages, and libraries that are used with the generated code. i.e. Use what is suggested by default by Visual Studio and the analyzers that come with the language and frameworks used in/by the generated code.

1. We want the generated code to be consistent.
2. We don't want anyone to open a generated solution and see lots of warnings or suggestions.
3. We want what's generated to closely resemble what people will most likely see elsewhere. (i.e. documentation and other instructional/training resources.)
4. We don't want to be frequently debating how things should be structured or formatted.
5. We don't want to do anything that could be considered "non-standard" simply because it is the preference of the person authoring a template.

There may be cases where there are reasons to make exceptions. These should be fully documented for future reference and to avoid setting a precedent for inconsistency.

I think we can do this without needing to include a .editorconfig file in the generated project. If we did, it would just include all the default settings and so be of no great benefit.

I'm open to changing this if/when the templates provided by default within VS include such a file.

---

Adding a feature to include such a file, if selected within the wizard, is a possibility but I'll let someone else open such an issue when they have the need and justification for doing so. Visual Studio already makes adding such a file very simple and so I see little reason to prioritize this here.

---

Consistency of code in the repo code-base is currently enforced via StyleCop. Changing this to be done with editorconfig is a possibility but I'll wait before opening a separate issue to track that.

---

We currently have tests (for UWP & WPF?) to check that the generated code is consistent but the rules these use (via a custom StyleCop config) are not recommended for inclusion in the generated project as they are heavily influenced by the requirements of authoring templates. We should probably review how/if this is done with the WinUI templates.

---

changing tags to prompt later review with regard to WinUI3 & WPF6 templates. We'll not change the UWP templates for this.

[mikebattista]

This issue was from 2018 so maybe the context here is out of date. If StyleCop is already enforcing consistent guidelines in the repo then the original intent of this issue is resolved.

If StyleCop is enforcing code guidelines, what additional value are the tests adding?

[mrlacey]

If StyleCop is enforcing code guidelines, what additional value are the tests adding?

The way the test works is by generating the apps form templates, adding StyleCop references & config (via a special template only available in the tests) and then building the generated app with warnings set as errors so code inconsistency issues fail the build and therefore the test.

[mikebattista]

This is being implemented as part of https://github.com/microsoft/TemplateStudio/pull/4490. Default CA rules are complaining about inconsistencies like this, and I believe Template Studio should have 0 CA warnings out of the box.

I've added an .editorconfig to generated projects which represents default rules but with increased severity for the CA violations we are fixing as well as for applying expression bodies more consistently for properties, accessors, and possibly methods as well. This should help us identify warnings during development of the templates, help developers maintain those standards in their generated projects, and also potentially could be used to fail our test passes if any warnings are reported so we don't introduce new CA violations going forward.

[mrlacey]

For additional background, the original UWP templates did not include a .editorconfig file although it was considered/discussed several times. The reason for not including one was the complexity of managing all the different combinations of templates (including across multiple frameworks) made getting the generated code to always match what the compiler/config wanted and expected meant the templates would get much more complicated.

As the WinUI (C#) functionality is enhanced this is something to be aware of. As long as modifications aren't made to the config file to match what is output (or to make writing the templates easier) then this should be a good thing.

[mikebattista]

Merged with #4490.