Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue215 #228

Merged
merged 5 commits into from
Nov 25, 2018
Merged

Issue215 #228

merged 5 commits into from
Nov 25, 2018

Conversation

david-garcia-garcia
Copy link
Contributor

@david-garcia-garcia david-garcia-garcia commented Nov 25, 2018

2 Changes in this PR.

  • DynamicClassFactory.cs no longer creates a parameter constructor when the class has no properties. Prior to this, a class with a duplicate empty constructor was created.

  • Added DisableMemberAccessToIndexAccessorFallback to disable automatic downcast/fallback to item accesor when a member is not found. This is an enhancement for developers because it helps early detection of badly created expressions that will fail to evaluate. Previous behaviour is respected as this is a new optional flag.

Copy link
Collaborator

@StefH StefH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some minor textual changes

{
return Expression.Call(instance, indexerMethod, Expression.Constant(id));
MethodInfo indexerMethod = instance.Type.GetMethod("get_Item", new[] {typeof(string)});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing some spaces here, did you do a format-file?

@@ -103,5 +103,12 @@ public IExpressionPromoter ExpressionPromoter
/// Renames the (Typed)ParameterExpression empty Name to a the correct supplied name from `it`. Default value is false.
/// </summary>
public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based

@@ -103,5 +103,12 @@ public IExpressionPromoter ExpressionPromoter
/// Renames the (Typed)ParameterExpression empty Name to a the correct supplied name from `it`. Default value is false.
/// </summary>
public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double 's'

/// <summary>
/// By default when a member is not found in a type and the type has a string bassed index accessor it will be parsed as an index accesor. Use
/// this flag to disable this behaviour and have parsing fail when parsing an expression
/// where a member access on a non existing member happens
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// where a member access on a non existing member happens
/// where a member access on a non existing member happens. Default value is false.

@StefH
Copy link
Collaborator

StefH commented Nov 25, 2018

It seems I did forget the CI Pull Request build integration with Azure Pipelines. I did change this and I hope that when you push new code, the build will trigger...

@StefH StefH added the feature label Nov 25, 2018
@codecov
Copy link

codecov bot commented Nov 25, 2018

Codecov Report

Merging #228 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
+ Coverage   96.63%   96.63%   +<.01%     
==========================================
  Files          41       41              
  Lines        6648     6654       +6     
==========================================
+ Hits         6424     6430       +6     
  Misses        224      224
Impacted Files Coverage Δ
...ystem.Linq.Dynamic.Core/Parser/ExpressionParser.cs 98.58% <100%> (ø) ⬆️
...rc/System.Linq.Dynamic.Core/DynamicClassFactory.cs 100% <100%> (ø) ⬆️
src/System.Linq.Dynamic.Core/ParsingConfig.cs 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c814eb2...5141da3. Read the comment docs.

@@ -103,5 +103,12 @@ public IExpressionPromoter ExpressionPromoter
/// Renames the (Typed)ParameterExpression empty Name to a the correct supplied name from `it`. Default value is false.
/// </summary>
public bool RenameParameterExpression { get; set; } = false;

/// <summary>
/// By default when a member is not found in a type and the type has a string based index accessor it will be parsed as an index accesor. Use
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accessor

@StefH
Copy link
Collaborator

StefH commented Nov 25, 2018

1 more typo in accessor

still a typo in based

@david-garcia-garcia
Copy link
Contributor Author

Fixed the typo in accessor, still not sure what the typo in "based" is.

@StefH
Copy link
Collaborator

StefH commented Nov 25, 2018

all ok

@StefH
Copy link
Collaborator

StefH commented Nov 25, 2018

linked to #215

@StefH StefH added the bug label Nov 25, 2018
@StefH StefH merged commit 1ee8997 into zzzprojects:master Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants