Keep the error location when the error comes from inside Pester, and name the assertion

src/functions/Output.ps1

@@ -454,12 +454,9 @@ function ConvertTo-FailureLines {
             # omit the lines internal to Pester
             if ((GetPesterOS) -ne 'Windows') {
                 [String]$isPesterFunction = '^at .*, .*/Pester.psm1: line [0-9]*$'
-                [String]$isShould = '^at (Should<End>|Invoke-Assertion), .*/Pester.psm1: line [0-9]*$'
-                # [String]$pattern6 = '^at <ScriptBlock>, (<No file>|.*/Pester.psm1): line [0-9]*$'
             }
             else {
                 [String]$isPesterFunction = '^at .*, .*\\Pester.psm1: line [0-9]*$'
-                [String]$isShould = '^at (Should<End>|Invoke-Assertion), .*\\Pester.psm1: line [0-9]*$'
             }
 
             # PESTER_BUILD
@@ -468,26 +465,42 @@ function ConvertTo-FailureLines {
                 # non inlined scripts will have different paths just omit everything from the src folder
                 $path = [regex]::Escape(($PSScriptRoot | & $SafeCommands['Split-Path']))
                 [String]$isPesterFunction = "^at .*, .*$path.*: line [0-9]*$"
-                [String]$isShould = "^at (Should<End>|Invoke-Assertion), .*$path.*: line [0-9]*$"
             }
             # end PESTER_BUILD
 
             # reducing the stack trace so we see only stack trace until the current It block and not up until the invocation of the
-            # whole test script itself. This is achieved by shortening the stack trace when any Runtime function is hit.
-            # what we don't want to do here is shorten the stack on the Should or Invoke-Assertion. That would remove any
-            # lines describing potential functions that are invoked in the test. e.g. doing function a() { 1 | Should -Be 2 }; a
-            # we want to be able to see that we invoked the assertion inside of function a
-            # the internal calls to Should and Invoke-Assertion are filtered out later by the second match
+            # whole test script itself. This is achieved by shortening the stack trace when a Pester frame is hit after we already
+            # collected at least one frame from the user code.
+            # the frames below the user code are Pester as well, e.g. Should, or Ensure-ExpectedIsNotCollection when the error is
+            # thrown from an assertion, or Mock when the error comes from Mock. We skip those, but we must not stop on them,
+            # otherwise we throw away the whole trace, including the line in the test file that the user needs. Skipping instead of
+            # stopping also keeps any function that the user invoked in the test, e.g. doing function a() { 1 | Should -Be 2 }; a
+            # shows that we invoked the assertion inside of function a.
+            # an assertion failure already has the line with the assertion itself, taken from TargetObject above,
+            # skip the first frame of the trace when it points at the same place so we don't print it twice
+            $skipFrame = if ($ErrorRecord.FullyQualifiedErrorId -eq 'PesterAssertionFailed') {
+                "$($ErrorRecord.TargetObject.File):$($ErrorRecord.TargetObject.Line)"
+            }
+
+            $userFrameFound = $false
             foreach ($line in $traceLines) {
-                if ($line -match $isPesterFunction -and $line -notmatch $isShould) {
-                    break
+                if ($line -match $isPesterFunction) {
+                    if ($userFrameFound) {
+                        break
+                    }
+
+                    continue
                 }
 
-                $isPesterInternalFunction = $line -match $isPesterFunction
+                if (-not $userFrameFound) {
+                    $userFrameFound = $true
 
-                if (-not $isPesterInternalFunction) {
-                    $lines.Trace += $line
+                    if ($null -ne $skipFrame -and $line -replace '^at [^,]*, ' -replace ':\s*line\s*(\d+)\s*$', ':$1' -eq $skipFrame) {
+                        continue
+                    }
                 }
+
+                $lines.Trace += $line
             }
         }
 

src/functions/assert/Common/Ensure-ExpectedIsNotCollection.ps1

@@ -1,11 +1,16 @@
 function Ensure-ExpectedIsNotCollection {
     param(
-        $InputObject
+        $InputObject,
+        # Name of the assertion that is doing the check, so the message can say which assertion refused
+        # the collection. The stack trace shows the line in the test file, not the assertion, so without
+        # the name here there is nothing that tells the user which assertion complained.
+        [string] $Assertion
     )
 
     if (Is-Collection $InputObject)
     {
-        throw [ArgumentException]'You provided a collection to the -Expected parameter. Using a collection on the -Expected side is not allowed by this assertion, because it leads to unexpected behavior. To compare collections use Should-BeCollection, or a more specialized collection assertion such as Should-Any or Should-All.'
+        $by = if ([string]::IsNullOrWhiteSpace($Assertion)) { 'this assertion' } else { $Assertion }
+        throw [ArgumentException]"You provided a collection to the -Expected parameter. Using a collection on the -Expected side is not allowed by $by, because it leads to unexpected behavior. To compare collections use Should-BeCollection, or a more specialized collection assertion such as Should-Any or Should-All."
     }
 
     $InputObject

src/functions/assert/Common/New-ShouldAssertion.ps1

@@ -108,9 +108,10 @@ class ShouldAssertion {
     }
 
     # Returns $Expected unchanged, or throws when it is a collection. Guards assertions that only
-    # make sense against a single value.
+    # make sense against a single value. The caller's name goes into the message, so the user sees
+    # which assertion refused the collection.
     [object] EnsureScalar([object] $Expected) {
-        return (Ensure-ExpectedIsNotCollection $Expected)
+        return (Ensure-ExpectedIsNotCollection -InputObject $Expected -Assertion $this.Caller.MyInvocation.MyCommand.Name)
     }
 
     # Returns whether a value is treated as a collection.

src/functions/assert/General/Should-BeSame.ps1

@@ -55,7 +55,7 @@
         [String]$Because
     )
 
-    $null = Ensure-ExpectedIsNotCollection $Expected
+    $null = Ensure-ExpectedIsNotCollection -InputObject $Expected -Assertion $PSCmdlet.MyInvocation.MyCommand.Name
 
     if ($Expected -is [ValueType] -or $Expected -is [string]) {
         throw [ArgumentException]"Should-BeSame compares objects by reference. You provided a value type or a string, those are not reference types and you most likely don't need to compare them by reference, see https://github.com/nohwnd/Assert/issues/6.`n`nAre you trying to compare two values to see if they are equal? Use Should-BeEqual instead."

src/functions/assert/General/Should-NotBeSame.ps1

@@ -54,7 +54,7 @@
         [String]$Because
     )
 
-    $null = Ensure-ExpectedIsNotCollection $Expected
+    $null = Ensure-ExpectedIsNotCollection -InputObject $Expected -Assertion $PSCmdlet.MyInvocation.MyCommand.Name
 
     $assert = New-ShouldAssertion -Caller $PSCmdlet -Actual $Actual -Buffer $local:Input
     $Actual = $assert.Actual()

tst/functions/Output.Tests.ps1

@@ -376,6 +376,81 @@ InModuleScope -ModuleName Pester -ScriptBlock {
                 $r.Trace.Count | Should -Be 1
             }
         }
+
+        Context 'error thrown from inside a Pester function' {
+            BeforeAll {
+                $testPath = Join-Path $TestDrive test.ps1
+                Set-Content -Path $testPath -Value @'
+1, 2, 3 | Should-Be 1, 2, 3
+'@
+
+                try {
+                    & $testPath
+                }
+                catch {
+                    $e = $_
+                }
+
+                $r = $e | ConvertTo-FailureLines
+            }
+
+            It 'keeps the line from the test file.' {
+                $r.Trace[0] | Should -Be "at <ScriptBlock>, ${testPath}:1"
+            }
+
+            It 'drops the frames that are inside Pester.' {
+                $r.Trace -join [Environment]::NewLine | Should -Not -Match 'Should-Be|EnsureScalar|Ensure-ExpectedIsNotCollection'
+            }
+        }
+
+        Context 'assertion fails inside a function defined in file' {
+            BeforeAll {
+                $testPath = Join-Path $TestDrive test.ps1
+                Set-Content -Path $testPath -Value @'
+function f1 {
+    1 | Should-Be 2
+}
+f1
+'@
+
+                try {
+                    & $testPath
+                }
+                catch {
+                    $e = $_
+                }
+
+                $r = $e | ConvertTo-FailureLines
+            }
+
+            It 'produces the line with the assertion, and the line that called the function.' {
+                $r.Trace[0] | Should -Be "at 1 | Should-Be 2, ${testPath}:2"
+                $r.Trace[1] | Should -Be "at <ScriptBlock>, ${testPath}:4"
+            }
+        }
+
+        Context 'assertion fails directly in file' {
+            BeforeAll {
+                $testPath = Join-Path $TestDrive test.ps1
+                Set-Content -Path $testPath -Value @'
+1 | Should-Be 2
+'@
+
+                try {
+                    & $testPath
+                }
+                catch {
+                    $e = $_
+                }
+
+                $r = $e | ConvertTo-FailureLines
+            }
+
+            It 'does not print the same file and line twice.' {
+                $r.Trace[0] | Should -Be "at 1 | Should-Be 2, ${testPath}:1"
+                @($r.Trace -match ([regex]::Escape("${testPath}:1"))).Count | Should -Be 1
+            }
+        }
     }
 
     Describe Format-ErrorMessage {
@@ -387,6 +462,8 @@ InModuleScope -ModuleName Pester -ScriptBlock {
                 catch [System.DivideByZeroException] {
                     $errorRecord = $_
                 }
+                # the line above where 1/0 is, taken from the record so adding tests to this file does not break it
+                $divideLine = $errorRecord.InvocationInfo.ScriptLineNumber
                 $errorRecord | Add-Member -Name "DisplayErrorMessage" -MemberType NoteProperty -Value "Failed to divide 1/0"
 
                 $stackTraceText = $errorRecord.Exception.ToString() + "$([Environment]::NewLine)at <ScriptBlock>, ${PSCommandPath}:230"
@@ -431,7 +508,7 @@ InModuleScope -ModuleName Pester -ScriptBlock {
                 $errorMessage = Format-ErrorMessage -Err $errorRecord -StackTraceVerbosity $_
                 $messages = $errorMessage -split [Environment]::NewLine
                 $messages[0] | Should -BeExactly "System.DivideByZeroException: Attempted to divide by zero."
-                $messages[1] | Should -BeExactly "at <ScriptBlock>, ${PSCommandPath}: line 385"
+                $messages[1] | Should -BeExactly "at <ScriptBlock>, ${PSCommandPath}: line $divideLine"
                 $messages.Count | Should -BeGreaterThan 1
             }
 

tst/functions/assert/Common/Ensure-ExpectedIsNotCollection.Tests.ps1

@@ -12,6 +12,11 @@ InPesterModuleScope {
             $err.Exception.Message | Verify-Equal 'You provided a collection to the -Expected parameter. Using a collection on the -Expected side is not allowed by this assertion, because it leads to unexpected behavior. To compare collections use Should-BeCollection, or a more specialized collection assertion such as Should-Any or Should-All.'
         }
 
+        It "Given a collection and an assertion name it names the assertion in the message" {
+            $err = { Ensure-ExpectedIsNotCollection -InputObject @() -Assertion 'Should-Be' } | Verify-Throw
+            $err.Exception.Message | Verify-Equal 'You provided a collection to the -Expected parameter. Using a collection on the -Expected side is not allowed by Should-Be, because it leads to unexpected behavior. To compare collections use Should-BeCollection, or a more specialized collection assertion such as Should-Any or Should-All.'
+        }
+
 
         It "Given a value it passes it to output when it is not a collection" {
             Ensure-ExpectedIsNotCollection -InputObject 'a' | Verify-Equal 'a'