Cleanup and organize some code

[parlough]

There isn't any functional changes here, mostly just cleanup and standardization:

• Switching to const when appropriate
• Switching to final if field isn't reassigned
• Switching from a list to a set if just used for a contains check
• Reorganizes some constructors and fields to be consistent
• Minor miscellaneous changes

[pq]

Thanks for this!

I need to simmer on a few suggestions and cycle back w/ some other contributors who are out over the holiday but a few quick thoughts.

• Switching to const when appropriate
• Switching to final if field isn't reassigned

+1

• Switching from a list to a set if just used for a contains check

I'm having deja vu but can't remember the substance of a conversation I had a while back with @bwilkerson about this. Brian: do you recall?

• Reorganizes some constructors and fields to be consistent

Though it's not enforced, we do use the analyzer's "Sort Members" command. I'd like to see this enforced in the build (dart-lang/sdk#58284) but that's tricky since the bits we'd want to repurpose are in the analysis server package (which is not published). Could you characterize the consistency you're after? (And thanks for the care!)

• Minor miscellaneous changes

[parlough]

I'm having deja vu but can't remember the substance of a conversation I had a while back with @bwilkerson about this. Brian: do you recall?

I am also curious about this, so I'll wait for the input from Brian. In the end, if so desired, I can switch these back to lists.

Though it's not enforced, we do use the analyzer's "Sort Members" command. I'd like to see this enforced in the build (#2368) but that's tricky since the bits we'd want to repurpose are in the analysis server package (which is not published). Could you characterize the consistency you're after? (And thanks for the care!)

That would definitely be nice to have. I simply moved a few constructors for visitors below the field declarations as that seemed to be the common standard elsewhere in the code, and it is what makes sense to me.

Minor miscellaneous changes

If you were curious what these were, they were mostly to enable some of the final changes. For example I switched this line:
environment = environment.where((e) => e.target != node).toList();
to this:
environment.removeWhere((e) => e.target == node);

---

I also added the int lineNumber optional parameter to the Annotation.forLint constructor as the only time the constructor was used, set the lineNumber with a cascade immediately afterwards.

[pq]

Thanks for your continued patience on this one. I'm working on migrating to NNBD (#2399) and would prefer to wait for that to land. If that stalls we can revisit?

[parlough]

Thanks for your continued patience on this one. I'm working on migrating to NNBD (#2399) and would prefer to wait for that to land. If that stalls we can revisit?

@pq Sounds good to me. Would you mind CCing me on that PR once you make it? I'd like to make a pass over it.

[pq]

sure thing. My guess is the first one may be imperfect and we can tighten some things over a few passes. Input will be welcome!

[parlough]

Will revisit this after the nnbd migration if necessary.