Fixes #3192. Improve correctness / clarity of existing View.AutoSize functionality/unit tests

[tig]

Fixes

• Fixes Improve correctness / clarity of existing View.AutoSize functionality/unit tests  #3192

Proposed Changes/Todos

• Rename/Move AutoSize related unit tests to a) make more clear, b) help diagnose issues by having all tests that test AutoSize = true in one place.
• Enforce that it makes no sense to set Width/Height if AutoSize = true. Update Unit tests to match.
• Modernize AutoSize related unit tests to remove dependency on Application and Label
• Create a new set of AutoSize = true unit tests that ensure full understanding of existing capabilities
• View with AutoSize = true and Label defaults to dims of 0,0 if Text is empty. Button defaults to 5, 1 (⟦► ◄⟧).
• Change TextFormatter.Lines_get to NOT cause a format. Callers should call Format explicitly. Property getters should not cause side effects. In this case, it causes all sorts of mayhem when debugging TextFormatter because the debugger display does Lines_get all the time.
• Make View.TextFormatter be init only.

Pull Request checklist:

• I've named my PR in the form of "Fixes #issue. Terse description."
• My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• My code follows the Terminal.Gui library design guidelines
• I ran dotnet test before commit
• I have made corresponding changes to the API documentation (using /// style comments)
• My changes generate no new warnings
• I have checked my code and corrected any poor grammar or misspellings
• I conducted basic QA to assure all features are working

[dodexahedron]

And, just to be clear, any I didn't mention ReSharper for aren't necessarily in scope for this and are just notes.

I'll check this branch out and make rule changes to address and push those so you can pull them in and keep the reformat activity consolidated here.

[dodexahedron]

Hmmm...

Well, I made the rules for null check pattern, but the code cleanup isn't actually applying them in all places.

I suspect it just backs off for safety, if the control flow statement is more than JUST the null check by itself, but that's a guess based on what I'm observing.

We'll probably have to address those manually, unless we can find a way to make it do it automatically in more places.

[dodexahedron]

There ya go.

Sent you a PR on your branch for a small set of rules

[dodexahedron]

If you run that on your machine for a file that got the order of fields and stuff wrong, does it look like it at least resulted in improving that situation?

I'm not very optimistic about it fixing many or maybe even any of the remaining null checks automatically.

I'm actually filing a bug/suggestion regarding that, because they seem like such simple and obvious cases for it to handle and I'm surprised it's not doing it.

[dodexahedron]

Oh and note the profile to use is the "All Rules" profile, because the default profiles ignore code that is excluded by current preprocessor evaluations. This profile applies the same logic to them, as well, regardless of how they evaluate.

[dodexahedron]

I've got some more tweaks queued up for the settings, but they're mostly for inspections and thus aren't directly related to this work, since they won't result in a change in code cleanup behavior. They're just for providing additional analyzer tool tips and refactoring actions that can be used or ignored when working on something.

As we make things better and better, we should probably consider turning the severity of the more valuable ones higher. Maybe after we declare v2 solidly beta quality or at whatever point we call a feature freeze?

[dodexahedron]

Saw the push.

I'll go back over again in a few minutes.

The light at the end of the tunnel is getting closer!

[tig]

NOW. ;-)

[dodexahedron]

Nice.

It looks like it took care of a lot of the field re-ordering, with one exception I'll have to fiddle with to get it to behave properly.

The layout rules do specify putting backing fields by their properties, and that rule has a high priority, so I'm not entirely sure without poking it some more why it isn't doing that specific bit.

Otherwise, this is looking a lot better so far. :)

[dodexahedron]

That one is so minor, though, that I'm kinda inclined to say, unless we can figure it out in short order, we can leave that for later. The diffs caused by that when code with that issue is touched are just single-line moves, and so are easy to eyeball.

[tig]

I'm done with this. Merging!! :-)

[tig]

We can do that in another PR.

I'm shutting down for the superbowl. Cheers!

[dodexahedron]

Haha good plan all around.

Have fun!

[dodexahedron]

And also, thanks for tackling this monster!

It's rare that something this big gets such a big purely code-quality changeset like this. Greatly appreciated. 😊

I'll make a new issue/PR for the editorconfig stuff and take a quick pass over the docs to be sure they match what we actually did, in the end.