Skip to content

Fix false positive for UsingInternalURL #255

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

Merged
merged 1 commit into from
Jun 26, 2015
Merged

Fix false positive for UsingInternalURL #255

merged 1 commit into from
Jun 26, 2015

Conversation

yutingc
Copy link
Contributor

@yutingc yutingc commented Jun 25, 2015

Add check if -replace is used and check if invocation expression is
[String] or [System.String]. This is to address issue #198

Add check if -replace is used and check if invocation expression is
[String] or [System.String]
@raghushantha
Copy link
Member

Hi Yuting.

Does this change address the issue reported by Dan (using composite formatting strings)?
https://msdn.microsoft.com/en-us/library/txafckwd(v=vs.110).aspx

-Update-
looks like it does. never mind

@raghushantha
Copy link
Member

looks good..

@@ -44,8 +46,19 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
foreach (StringConstantExpressionAst expressionAst in expressionAsts)
{
//Check if XPath is used. If XPath is used, then we don't throw warnings.

Ast parentAst = expressionAst.Parent;

Choose a reason for hiding this comment

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

Is this gonna raise a NullPointerException if parentAst is null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it will ever be null..At least the root Ast should be a ScriptBlockAst?

Choose a reason for hiding this comment

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

Have you tried on a script with only a string as its content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah..I tried with a single line script and it didn't throw warnings or errors.

yutingc added a commit that referenced this pull request Jun 26, 2015
Fix false positive for UsingInternalURL
@yutingc yutingc merged commit 96d56c3 into BugFixes Jun 26, 2015
@vors vors removed the in progress label Jun 26, 2015
@yutingc yutingc deleted the internalURL branch June 26, 2015 17:48
@yutingc
Copy link
Contributor Author

yutingc commented Jun 26, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants