From ea99c42df6f0cd8d6bc6e9014948cb5345035aea Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@craftyjon.com>
Date: Sun, 2 Jan 2022 13:19:31 -0500
Subject: [PATCH] Move duplicate field handling into field parser

Fixes https://gitlab.com/kicad/code/kicad/-/issues/10130
---
 .../sch_plugins/kicad/sch_sexpr_parser.cpp    | 42 ++++++++-----------
 eeschema/sch_plugins/kicad/sch_sexpr_parser.h |  3 ++
 2 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp
index 9e15c57f74..fbe9d4ad8f 100644
--- a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp
+++ b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp
@@ -162,14 +162,13 @@ LIB_SYMBOL* SCH_SEXPR_PARSER::ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, int aF
     wxString name;
     wxString error;
     LIB_ITEM* item;
-    LIB_FIELD* field;
     std::unique_ptr<LIB_SYMBOL> symbol = std::make_unique<LIB_SYMBOL>( wxEmptyString );
-    std::set<int> fieldIDsRead;
 
     m_requiredVersion = aFileVersion;
     symbol->SetUnitCount( 1 );
 
     m_fieldId = MANDATORY_FIELDS;
+    m_fieldIDsRead.clear();
 
     token = NextTok();
 
@@ -232,29 +231,7 @@ LIB_SYMBOL* SCH_SEXPR_PARSER::ParseSymbol( LIB_SYMBOL_MAP& aSymbolLibMap, int aF
             break;
 
         case T_property:
-            field = parseProperty( symbol );
-
-            if( field )
-            {
-                // Due to an bug when in #LIB_SYMBOL::Flatten, duplicate ids slipped through
-                // when writing files.  This section replaces duplicate #LIB_FIELD indices on
-                // load.
-                if( fieldIDsRead.count( field->GetId() ) )
-                {
-                    int nextAvailableId = field->GetId() + 1;
-
-                    while( fieldIDsRead.count( nextAvailableId ) )
-                        nextAvailableId += 1;
-
-                    fieldIDsRead.insert( nextAvailableId );
-                    field->SetId( nextAvailableId );
-                }
-                else if( field )
-                {
-                    fieldIDsRead.insert( field->GetId() );
-                }
-            }
-
+            parseProperty( symbol );
             break;
 
         case T_extends:
@@ -836,6 +813,19 @@ LIB_FIELD* SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_SYMBOL>& aSymbol
         }
     }
 
+    // Due to an bug when in #LIB_SYMBOL::Flatten, duplicate ids slipped through
+    // when writing files.  This section replaces duplicate #LIB_FIELD indices on
+    // load.
+    if( m_fieldIDsRead.count( field->GetId() ) )
+    {
+        int nextAvailableId = field->GetId() + 1;
+
+        while( m_fieldIDsRead.count( nextAvailableId ) )
+            nextAvailableId += 1;
+
+        field->SetId( nextAvailableId );
+    }
+
     LIB_FIELD* existingField;
 
     if( field->GetId() < MANDATORY_FIELDS )
@@ -843,6 +833,7 @@ LIB_FIELD* SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_SYMBOL>& aSymbol
         existingField = aSymbol->GetFieldById( field->GetId() );
 
         *existingField = *field;
+        m_fieldIDsRead.insert( field->GetId() );
         return existingField;
     }
     else if( name == "ki_keywords" )
@@ -908,6 +899,7 @@ LIB_FIELD* SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_SYMBOL>& aSymbol
         if( !existingField )
         {
             aSymbol->AddDrawItem( field.get(), false );
+            m_fieldIDsRead.insert( field->GetId() );
             return field.release();
         }
         else
diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_parser.h b/eeschema/sch_plugins/kicad/sch_sexpr_parser.h
index 0cd7239430..5b57ce2c27 100644
--- a/eeschema/sch_plugins/kicad/sch_sexpr_parser.h
+++ b/eeschema/sch_plugins/kicad/sch_sexpr_parser.h
@@ -81,6 +81,9 @@ class SCH_SEXPR_PARSER : public SCHEMATIC_LEXER
     int m_convert;          ///< The current body style being parsed.
     wxString m_symbolName;  ///< The current symbol name.
 
+    /// Field IDs that have been read so far for the current symbol.
+    std::set<int>      m_fieldIDsRead;
+
     std::set<KIID>     m_uuids;
 
     PROGRESS_REPORTER* m_progressReporter;  // optional; may be nullptr