Throw duplicate pin error as it prevents other checks.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/14761

eeschema/erc/erc.cpp

@@ -131,6 +131,9 @@ const std::set<ELECTRICAL_PINTYPE> DrivenPinTypes =
             ELECTRICAL_PINTYPE::PT_POWER_IN
         };
 
+extern void CheckDuplicatePins( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
+                                UNITS_PROVIDER* aUnitsProvider );
+
 int ERC_TESTER::TestDuplicateSheetNames( bool aCreateMarker )
 {
     SCH_SCREEN* screen;
@@ -1063,8 +1066,24 @@ int ERC_TESTER::TestLibSymbolIssues()
             std::unique_ptr<LIB_SYMBOL> flattenedSymbol = libSymbol->Flatten();
             constexpr int flags = SCH_ITEM::COMPARE_FLAGS::EQUALITY | SCH_ITEM::COMPARE_FLAGS::ERC;
 
-            if( settings.IsTestEnabled( ERCE_LIB_SYMBOL_MISMATCH )
+            if( settings.IsTestEnabled( ERCE_LIB_SYMBOL_MISMATCH ) )
-                && flattenedSymbol->Compare( *libSymbolInSchematic, flags ) != 0 )
+            {
+                // We have to check for duplicate pins first as they will cause Compare() to fail.
+                std::vector<wxString> messages;
+                UNITS_PROVIDER        unitsProvider( schIUScale, EDA_UNITS::MILS );
+                CheckDuplicatePins( libSymbolInSchematic, messages, &unitsProvider );
+
+                if( !messages.empty() )
+                {
+                    std::shared_ptr<ERC_ITEM> ercItem = ERC_ITEM::Create( ERCE_DUPLICATE_PIN_ERROR );
+                    ercItem->SetItems( symbol );
+                    msg.Printf( _( "Symbol '%s' has multiple pins with the same pin number" ),
+                                UnescapeString( symbolName ) );
+                    ercItem->SetErrorMessage( msg );
+
+                    markers.emplace_back( new SCH_MARKER( ercItem, symbol->GetPosition() ) );
+                }
+                else if( flattenedSymbol->Compare( *libSymbolInSchematic, flags ) != 0 )
                 {
                     std::shared_ptr<ERC_ITEM> ercItem = ERC_ITEM::Create( ERCE_LIB_SYMBOL_MISMATCH );
                     ercItem->SetItems( symbol );
@@ -1076,6 +1095,7 @@ int ERC_TESTER::TestLibSymbolIssues()
                     markers.emplace_back( new SCH_MARKER( ercItem, symbol->GetPosition() ) );
                 }
             }
+        }
 
         for( SCH_MARKER* marker : markers )
         {

eeschema/erc/erc_item.cpp

@@ -63,6 +63,10 @@ ERC_ITEM ERC_ITEM::powerpinNotDriven( ERCE_POWERPIN_NOT_DRIVEN,
         _( "Input Power pin not driven by any Output Power pins" ),
         wxT( "power_pin_not_driven" ) );
 
+ERC_ITEM ERC_ITEM::duplicatePinError( ERCE_DUPLICATE_PIN_ERROR,
+        _( "Multiple pins with the same pin numbers" ),
+        wxT( "duplicate_pins" ) );
+
 ERC_ITEM ERC_ITEM::pinTableWarning( ERCE_PIN_TO_PIN_WARNING,
         _( "Conflict problem between pins" ),
         wxT( "pin_to_pin" ) );
@@ -263,6 +267,7 @@ std::shared_ptr<ERC_ITEM> ERC_ITEM::Create( int aErrorCode )
     case ERCE_PIN_NOT_CONNECTED:       return std::make_shared<ERC_ITEM>( pinNotConnected );
     case ERCE_PIN_NOT_DRIVEN:          return std::make_shared<ERC_ITEM>( pinNotDriven );
     case ERCE_POWERPIN_NOT_DRIVEN:     return std::make_shared<ERC_ITEM>( powerpinNotDriven );
+    case ERCE_DUPLICATE_PIN_ERROR:     return std::make_shared<ERC_ITEM>( duplicatePinError );
     case ERCE_PIN_TO_PIN_WARNING:      return std::make_shared<ERC_ITEM>( pinTableWarning );
     case ERCE_PIN_TO_PIN_ERROR:        return std::make_shared<ERC_ITEM>( pinTableError );
     case ERCE_HIERACHICAL_LABEL:       return std::make_shared<ERC_ITEM>( hierLabelMismatch );

eeschema/erc/erc_item.h

@@ -188,6 +188,7 @@ private:
     static ERC_ITEM pinNotConnected;
     static ERC_ITEM pinNotDriven;
     static ERC_ITEM powerpinNotDriven;
+    static ERC_ITEM duplicatePinError;
     static ERC_ITEM pinTableWarning;
     static ERC_ITEM pinTableError;
     static ERC_ITEM hierLabelMismatch;

eeschema/erc/erc_settings.cpp

@@ -241,10 +241,16 @@ ERC_SETTINGS::~ERC_SETTINGS()
 
 SEVERITY ERC_SETTINGS::GetSeverity( int aErrorCode ) const
 {
+    // Special-case duplicate pin error.  Unique pin names are required by KiCad, so this
+    // is always an error.
+    if( aErrorCode == ERCE_DUPLICATE_PIN_ERROR )
+    {
+        return RPT_SEVERITY_ERROR;
+    }
     // Special-case pin-to-pin errors:
     // Ignore-or-not is controlled by ERCE_PIN_TO_PIN_WARNING (for both)
     // Warning-or-error is controlled by which errorCode it is
-    if( aErrorCode == ERCE_PIN_TO_PIN_ERROR )
+    else if( aErrorCode == ERCE_PIN_TO_PIN_ERROR )
     {
         wxASSERT( m_ERCSeverities.count( ERCE_PIN_TO_PIN_WARNING ) );
 

eeschema/erc/erc_settings.h

@@ -89,6 +89,7 @@ enum ERCE_T
 
     // Errors after this point will not automatically appear in the Severities Panel
 
+    ERCE_DUPLICATE_PIN_ERROR,
     ERCE_PIN_TO_PIN_WARNING,    // pin connected to an other pin: warning level
     ERCE_PIN_TO_PIN_ERROR,      // pin connected to an other pin: error level
     ERCE_ANNOTATION_ACTION      // Not actually an error; just an action performed during

eeschema/symbol_checker.cpp

@@ -31,45 +31,12 @@
 static bool sort_by_pin_number( const SCH_PIN* ref, const SCH_PIN* tst );
 
 static void CheckLibSymbolGraphics( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
-                                     EDA_DRAW_FRAME* aUnitsProvider );
+                                    UNITS_PROVIDER* aUnitsProvider );
 
-/**
+void CheckDuplicatePins( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
- * Check a lib symbol to find incorrect settings
+                         UNITS_PROVIDER* aUnitsProvider )
- * Pins not on a valid grid
- * Pins duplicated
- * Conflict with pins at same location
- * Incorrect Power Symbols
- * illegal reference prefix (cannot ends by a digit or a '?')
- * @param aSymbol is the library symbol to check
- * @param aMessages is a room to store error messages
- * @param aGridForPins (in IU) is the grid to test pin positions ( >= 25 mils )
- * should be 25, 50 or 100 mils (convered to IUs)
- * @param aUnitsProvider a frame to format coordinates in messages
- */
-void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
-                     int aGridForPins, EDA_DRAW_FRAME* aUnitsProvider )
 {
-    if( !aSymbol )
-        return;
-
     wxString              msg;
-
-    // Test reference prefix validity:
-    // if the symbol is saved in a library, the prefix should not ends by a digit or a '?'
-    // but it is acceptable if the symbol is saved to aschematic
-    wxString reference_base = aSymbol->GetReferenceField().GetText();
-    wxString illegal_end( wxT( "0123456789?" ) );
-    wxUniChar last_char = reference_base.Last();
-
-    if( illegal_end.Find( last_char ) != wxNOT_FOUND )
-    {
-        msg.Printf( _( "<b>Warning: reference prefix</b><br>prefix ending by '%s' can create"
-                       " issues if saved in a symbol library" ),
-                    illegal_end );
-        msg += wxT( "<br><br>" );
-        aMessages.push_back( msg );
-    }
-
     std::vector<SCH_PIN*> pinList = aSymbol->GetPins();
 
     // Test for duplicates:
@@ -77,14 +44,6 @@ void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
     // (pins with the same number) will be consecutive in list
     sort( pinList.begin(), pinList.end(), sort_by_pin_number );
 
-    // The minimal grid size allowed to place a pin is 25 mils
-    // the best grid size is 50 mils, but 25 mils is still usable
-    // this is because all aSymbols are using a 50 mils grid to place pins, and therefore
-    // the wires must be on the 50 mils grid
-    // So raise an error if a pin is not on a 25 (or bigger :50 or 100) mils grid
-    const int min_grid_size = schIUScale.MilsToIU( 25 );
-    const int clamped_grid_size = ( aGridForPins < min_grid_size ) ? min_grid_size : aGridForPins;
-
     for( unsigned ii = 1; ii < pinList.size(); ii++ )
     {
         SCH_PIN* pin  = pinList[ii - 1];
@@ -181,6 +140,58 @@ void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
         msg += wxT( "<br><br>" );
         aMessages.push_back( msg );
     }
+}
+
+
+/**
+ * Check a lib symbol to find incorrect settings
+ * Pins not on a valid grid
+ * Pins duplicated
+ * Conflict with pins at same location
+ * Incorrect Power Symbols
+ * illegal reference prefix (cannot ends by a digit or a '?')
+ * @param aSymbol is the library symbol to check
+ * @param aMessages is a room to store error messages
+ * @param aGridForPins (in IU) is the grid to test pin positions ( >= 25 mils )
+ * should be 25, 50 or 100 mils (convered to IUs)
+ * @param aUnitsProvider a frame to format coordinates in messages
+ */
+void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
+                     int aGridForPins, UNITS_PROVIDER* aUnitsProvider )
+{
+    if( !aSymbol )
+        return;
+
+    wxString msg;
+
+    // Test reference prefix validity:
+    // if the symbol is saved in a library, the prefix should not ends by a digit or a '?'
+    // but it is acceptable if the symbol is saved to aschematic
+    wxString reference_base = aSymbol->GetReferenceField().GetText();
+    wxString illegal_end( wxT( "0123456789?" ) );
+    wxUniChar last_char = reference_base.Last();
+
+    if( illegal_end.Find( last_char ) != wxNOT_FOUND )
+    {
+        msg.Printf( _( "<b>Warning: reference prefix</b><br>prefix ending by '%s' can create"
+                       " issues if saved in a symbol library" ),
+                    illegal_end );
+        msg += wxT( "<br><br>" );
+        aMessages.push_back( msg );
+    }
+
+    CheckDuplicatePins( aSymbol, aMessages, aUnitsProvider );
+
+    std::vector<SCH_PIN*> pinList = aSymbol->GetPins();
+    sort( pinList.begin(), pinList.end(), sort_by_pin_number );
+
+    // The minimal grid size allowed to place a pin is 25 mils
+    // the best grid size is 50 mils, but 25 mils is still usable
+    // this is because all aSymbols are using a 50 mils grid to place pins, and therefore
+    // the wires must be on the 50 mils grid
+    // So raise an error if a pin is not on a 25 (or bigger :50 or 100) mils grid
+    const int min_grid_size = schIUScale.MilsToIU( 25 );
+    const int clamped_grid_size = ( aGridForPins < min_grid_size ) ? min_grid_size : aGridForPins;
 
     // Test for a valid power aSymbol.
     // A valid power aSymbol has only one unit, no alternate body styles and one pin.
@@ -353,7 +364,7 @@ void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
 
 
 void CheckLibSymbolGraphics( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
-                             EDA_DRAW_FRAME* aUnitsProvider )
+                             UNITS_PROVIDER* aUnitsProvider )
 {
     if( !aSymbol )
         return;

eeschema/tools/ee_inspection_tool.cpp

@@ -293,7 +293,7 @@ int EE_INSPECTION_TOOL::ExcludeMarker( const TOOL_EVENT& aEvent )
 
 
 extern void CheckLibSymbol( LIB_SYMBOL* aSymbol, std::vector<wxString>& aMessages,
-                           int aGridForPins, EDA_DRAW_FRAME* aUnitsProvider );
+                            int aGridForPins, UNITS_PROVIDER* aUnitsProvider );
 
 int EE_INSPECTION_TOOL::CheckSymbol( const TOOL_EVENT& aEvent )
 {