Remove dead code from binding#393

Merged

JuditRose merged 1 commit intodotnet:masterfrom

hughbe:listbindinghelper-bugs

Feb 20, 2019

Contributor

hughbe commented Jan 29, 2019

No description provided.


          Remove dead code from binding

53bd3f2

hughbe requested a review from a team as a code owner

January 29, 2019 00:50

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/BindingSource.cs

                       ///////////////////////////////////////////////////////////////////////////////
                       private void WireCurrencyManager(CurrencyManager cm) {
-                          if (cm != null) {

Contributor Author

hughbe Jan 29, 2019

As seen above, we can make currencyManager readonly as it is only allocated in the constructor. It can not be null

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/BindingSource.cs

                       }
                       private void UnwireCurrencyManager(CurrencyManager cm) {
-                          if (cm != null) {

Contributor Author

hughbe Jan 29, 2019

Same reasoning

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/BindingSource.cs

-                      private void DataSource_Initialized(object sender, EventArgs e) {
-                          ISupportInitializeNotification dsInit = (this.DataSource as ISupportInitializeNotification);
-                          Debug.Assert(dsInit != null, "BindingSource: ISupportInitializeNotification.Initialized event received, but current DataSource does not support ISupportInitializeNotification!");

Contributor Author

hughbe Jan 29, 2019

Broken debug assertions. Can occur when changing the data source on initialisation, or when initialization fails but EndInit is called

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/ListBindingHelper.cs

+                              return typedList.GetItemProperties(listAccessors);
+                          }
+                          else if (target is IEnumerable enumerable) {
+                              return GetListItemPropertiesByEnumerable(enumerable, listAccessors, 0);

Contributor Author

hughbe Jan 29, 2019

This is all formatting except for now we call GetListItemPropertiesByEnumerable(IEnumerable, PropertyDescriptor[], int) instead of the GetListItemPropertiesByEnumerable(IEnumerable, PropertyDescriptor[]) function which had dead code and does exactly the same thing when removing the dead code

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/ListBindingHelper.cs

-                      private static PropertyDescriptorCollection GetListItemPropertiesByType(Type type, PropertyDescriptor[] listAccessors) {
-                          PropertyDescriptorCollection pdc = null;
-                          if ((null == listAccessors) || (listAccessors.Length == 0)) {

Contributor Author

hughbe Jan 29, 2019

This could never be null or empty - demonstrated as this function is only called once in L165 which exits early if null or empty.

Once dead code is removed, the function can be inlined into single caller.

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/ListBindingHelper.cs

-                      private static PropertyDescriptorCollection GetListItemPropertiesByEnumerable(IEnumerable enumerable, PropertyDescriptor[] listAccessors) {
-                          PropertyDescriptorCollection pdc = null;
-                          if ((null == listAccessors) || (listAccessors.Length == 0)) {

Contributor Author

hughbe Jan 29, 2019

This could never be null or empty - demonstrated as this function is only called once in L165 which exits early if null or empty.

Once dead code is removed, the function can be inlined into single caller.

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/ListBindingHelper.cs

-                      private static PropertyDescriptorCollection GetListItemPropertiesByInstance(object target, PropertyDescriptor[] listAccessors, int startIndex) {
+                      private static PropertyDescriptorCollection GetListItemPropertiesByInstance(object target, PropertyDescriptor[] listAccessors, int startIndex)
+                      {
+                          Debug.Assert(listAccessors != null);

Contributor Author

hughbe Jan 29, 2019

Function only called from L168 which bails early for null or empty list accessors and GetListItemPropertiesByEnumerable which is called from L165 which as we know bailed early

hughbe commented

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/ListBindingHelper.cs

    
                                  pdc = TypeDescriptor.GetProperties(instance, BrowsableAttributeList);

                                  if (!(enumerable is IList) && (pdc == null || pdc.Count == 0)) {

                                  if (!(enumerable is IList) && pdc.Count == 0) {

Contributor Author

hughbe Jan 29, 2019

TypeDescriptor.GetProperties can never return null.

JuditRose approved these changes

View reviewed changes

Contributor Author

hughbe commented Feb 18, 2019

Good to merge @JuditRose ?

JuditRose merged commit d839421 into dotnet:master

hughbe deleted the listbindinghelper-bugs branch

February 20, 2019 18:07

zsd4yr pushed a commit that referenced this pull request


          Remove dead code from binding (#393)

888da65

JuditRose added a commit that referenced this pull request


          adding collection editor and object selector editor (#439)

43de673

* adding collection editor and object selector editor

* Moving ClientUtils and RTLAwareMessageBox to commons

* fixing PR comments, moving DpiAwernessContext to common, removing unneccessary (commented out) code and comments

* changing namespace and minor fixes requested in PR

* added new classes to ref

* remove ported components from api compat file

* replaced <devdoc> tags with <summary>, fixed a few minor PR comments

* changing using statement back

* adding auto generated resource files

* added unit tests for object selector editor

* adding collection editor unit tests

* add assertion to testcase

* fix namespacing

* index on WFI177-add-missing-editors: 81b9089 add assertion to testcase

* successfully added to ref assembly

* fix include in RTLAwareMessageBox and alphabetize usings in ClientUtils

* join all SR in the System namespace

* remove comment header from top of SR.resx files for System.Windows.Forms and System.Windows.Forms.Design.Editors; already done for System.Windows.Forms.Design

* Fix text in the ThreadExceptionDialog (#430)

Fixes #202 
* replaced hardcoded text that represents a resource name with a get …property that reads this resource
* replaced explicit \r\n strings in SR.RESX file for system.windows.forms.dll with newlines because resx reader escapes them and as a result we don't have new lines in the displayed text
* for other assemblies in this repo, I created a tracking issue to fix SR.resx files:
#429
* added a test for this dialog to `WinformsControlsTest`, @zsd4yr  - please review, I tested this change by running the app, is there anything else to run

Before:
![image](https://user-images.githubusercontent.com/15823268/52612877-6e9bce00-2e40-11e9-80a2-41efcf9b94ca.png)

After:
![image](https://user-images.githubusercontent.com/15823268/52612888-765b7280-2e40-11e9-988f-9501c75af58f.png)

* Update dependencies from https://github.com/dotnet/corefx build 20190213.23 (#444)

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19113.23
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19113.23
- Microsoft.Win32.Registry - 4.6.0-preview4.19113.23
- System.Windows.Extensions - 4.6.0-preview4.19113.23
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19113.23
- System.Security.Cryptography.Cng - 4.6.0-preview4.19113.23
- System.Security.Permissions - 4.6.0-preview4.19113.23
- System.Drawing.Common - 4.6.0-preview4.19113.23

* Update dependencies from https://github.com/dotnet/arcade build 20190213.1 (#443)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19113.1

* rename binlog (#440)

* rename Build.binlog

* we don't need out own anymore

* switch back for local build as well

* Exclude documentation files from NuGet package (#441)

* enable resource dlls in package (#445)

* Update .NET Core SDK link. (#442)

* Update .NET Core SDK link.

* Remove preview moniker from link text.

* Update the link text.

* Update dependencies from https://github.com/dotnet/corefx build 20190215.1 (#447)

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19115.1
- Microsoft.Win32.Registry - 4.6.0-preview4.19115.1
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19115.1
- System.Security.Cryptography.Cng - 4.6.0-preview4.19115.1
- System.Security.Permissions - 4.6.0-preview4.19115.1
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19115.1
- System.Windows.Extensions - 4.6.0-preview4.19115.1
- System.Drawing.Common - 4.6.0-preview4.19115.1

* Update dependencies from https://github.com/dotnet/arcade build 20190214.4 (#446)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19114.4

* Adding basic tests for AccessibleObjects (#435)

* Adding basic tests for ComboBoxAccessibleObject, DataGridViewAccessibleObject, PropertyGridAccessibleObject and ToolStripAccessibleObject.

* Make tested currentUICulture equal to currentCulture on the Thread (#453)

* enable resource dlls in package

* ensure ui culture is current culture

* Update dependencies from https://github.com/dotnet/arcade build 20190215.1 (#455)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19115.1

* [master] Update dependencies from dotnet/corefx (#456)

* Update dependencies from https://github.com/dotnet/corefx build 20190217.1

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19117.1
- Microsoft.Win32.Registry - 4.6.0-preview4.19117.1
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19117.1
- System.Security.Cryptography.Cng - 4.6.0-preview4.19117.1
- System.Security.Permissions - 4.6.0-preview4.19117.1
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19117.1
- System.Windows.Extensions - 4.6.0-preview4.19117.1
- System.Drawing.Common - 4.6.0-preview4.19117.1

* Update dependencies from https://github.com/dotnet/corefx build 20190218.1

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19118.1
- Microsoft.Win32.Registry - 4.6.0-preview4.19118.1
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19118.1
- System.Security.Cryptography.Cng - 4.6.0-preview4.19118.1
- System.Security.Permissions - 4.6.0-preview4.19118.1
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19118.1
- System.Windows.Extensions - 4.6.0-preview4.19118.1
- System.Drawing.Common - 4.6.0-preview4.19118.1

* Update dependencies from https://github.com/dotnet/corefx build 20190218.6

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19118.6
- Microsoft.Win32.Registry - 4.6.0-preview4.19118.6
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19118.6
- System.Security.Cryptography.Cng - 4.6.0-preview4.19118.6
- System.Security.Permissions - 4.6.0-preview4.19118.6
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19118.6
- System.Windows.Extensions - 4.6.0-preview4.19118.6
- System.Drawing.Common - 4.6.0-preview4.19118.6

* Fixed Clipboard.SetDataObject to work with custom formats #415 (#416)

fixes #415

While testing SpreadsheetGear for Windows Forms on .NET Core 3 we found that Clipboard.SetDataObject writes uninitialized data to the clipboard instead of serialized .NET objects.

This bug is apparently a regression from Span<T> work in 37abb3a.

* Remove dead code from binding (#393)

* Update dependencies from https://github.com/dotnet/arcade build 20190219.2 (#461)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19119.2

* LOC CHECKIN | dotnet/winforms master | 20190221 (#467)

* Update dependencies from https://github.com/dotnet/arcade build 20190221.1 (#466)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19121.1

* Set copyright and package license (#470)

* Update dependencies from https://github.com/dotnet/arcade build 20190221.5 (#474)

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19121.5

* [master] Update dependencies from dotnet/arcade (#481)

* Update dependencies from https://github.com/dotnet/arcade build 20190222.3

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19122.3

* Update dependencies from https://github.com/dotnet/arcade build 20190223.1

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19123.1

* Update dependencies from https://github.com/dotnet/arcade build 20190224.1

This change updates the following dependencies
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19124.1

* fix downgrade netcore platforms (#479)

* fix downgrade netcore platforms

* MicrosoftNETCorePlatforms is a CoreFX dependency

* change version to match other CoreFX dependencies; add to Version.Details.xml

* [master] Update dependencies from dotnet/corefx (#483)

* Update dependencies from https://github.com/dotnet/corefx build 20190223.2

This change updates the following dependencies
- System.CodeDom - 4.6.0-preview4.19123.2
- Microsoft.Win32.Registry - 4.6.0-preview4.19123.2
- Microsoft.Win32.SystemEvents - 4.6.0-preview4.19123.2
- System.Security.Cryptography.Cng - 4.6.0-preview4.19123.2
- System.Security.Permissions - 4.6.0-preview4.19123.2
- System.Configuration.ConfigurationManager - 4.6.0-preview4.19123.2
- System.Windows.Extensions - 4.6.0-preview4.19123.2
- System.Drawing.Common - 4.6.0-preview4.19123.2

* Update Version.Details.xml

* Fix InputLanguage null refs and add tests (#380)

* Fix InputLanguage null refs and add tests

* Update src/System.Windows.Forms/src/System/Windows/Forms/InputLanguage.cs

Co-Authored-By: hughbe <hughbellars@gmail.com>

* Save files as utf-8-bom (#487)

* Save files as UTF-8 (all files that did not change)

* Save files as UTF-8 (files with changes)

* Improve validation of Table arguments (#381)

* Added CheckParentingCycle, that check for any circular reference in the ancestors chain. (#454)

* Closes #358, Added CheckParentingCycle, that check for any circular reference in the ancestors chain.

* xlf resource files updated

* Refactored method CheckParentingCycle so that it has only one throw statement

* Removed reference to .uex file

* Removed character related to different encoding

* Added UnitTest to TreeView

To check the case of adding a circular reference to the ancestors chain

* Fix typo (#492)

* successfully added to ref assembly

* fix include in RTLAwareMessageBox and alphabetize usings in ClientUtils

* join all SR in the System namespace

* remove comment header from top of SR.resx files for System.Windows.Forms and System.Windows.Forms.Design.Editors; already done for System.Windows.Forms.Design

* typo

* it is unnecessary to generate source for Resources\CollectionEditor.resx

* add comments above public methods

* make internal as well as protected for tests

* FIxed interactive test for CollectionEditor

* add flimsy functional test:

* proper test helpers

hughbe mentioned this pull request

Cleanup Form.IsRestrictedWindow #695

Merged

ghost locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet