Skip to content
This repository was archived by the owner on Feb 4, 2019. It is now read-only.

Type comparison operators #420

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions Source/System.Management/Pash/Implementation/ExecutionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ object EvaluateBinaryExpression(BinaryExpressionAst binaryExpressionAst)
var right = LanguagePrimitives.ConvertTo<object[]>(rightOperand);
return string.Format(left, right);
}

case TokenKind.Is:
return IsInstanceOfType(leftOperand, rightOperand);
case TokenKind.IsNot:
return !IsInstanceOfType(leftOperand, rightOperand);
case TokenKind.Equals:
case TokenKind.PlusEquals:
case TokenKind.MinusEquals:
Expand All @@ -200,8 +203,6 @@ object EvaluateBinaryExpression(BinaryExpressionAst binaryExpressionAst)
case TokenKind.Cin:
case TokenKind.Cnotin:
case TokenKind.Csplit:
case TokenKind.Is:
case TokenKind.IsNot:
case TokenKind.As:
case TokenKind.Shl:
case TokenKind.Shr:
Expand All @@ -212,6 +213,17 @@ object EvaluateBinaryExpression(BinaryExpressionAst binaryExpressionAst)
}
}

private static bool IsInstanceOfType(object leftOperand, object rightOperand)
{
var type = rightOperand as Type;
Copy link
Contributor

@ygra ygra Jul 5, 2016

Choose a reason for hiding this comment

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

The right operand is coerced to a type, actually.

PS Home:\> '' -is 'string'
True

I think LanguagePrimitives.ConvertTo should take care of that.

And the right operand can be an expression:

'' -is [type]'string'
3 -is 'int'+32

Not sure right now whether that's already covered here.

Granted, we don't have to fix those in the first iteration, but we should keep it in mind.

if (type == null)
{
throw new InvalidOperationException(@"The right operand of '-is' must be a type.");
Copy link
Contributor

@ygra ygra Jul 5, 2016

Choose a reason for hiding this comment

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

Why a verbatim string literal here?

Copy link
Author

Choose a reason for hiding this comment

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

To emphasise localisable text only literal.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ygra, I think that some tools such as Resharper will throw a warning here mentioning "possibly localizable string". Making a literal verbatim is really a way to suppress this warning. So:

a) I don't think we generally use this practice in Pash code
b) that's really a localizable string, so it would be improper to just suppress the warning. If we one day will ultimately decide to localize Pash (what's unlikely but possible), we'll need to localize it.

So I don't think that you should use verbatim string literal here. Better get rid of that.

}

return type.IsInstanceOfType(leftOperand);
}

private bool Match(object leftOperand, object rightOperand, RegexOptions regexOptions)
{
if (!(leftOperand is string) || !(rightOperand is string))
Expand Down